feat: implement extra columns configuration in d3Map where the extra …

[timothygachengo]

…columns are used as variables for the tooltip

Description

implement extra columns configuration in d3Map for tooltips.
This should be only for single dimension values

Related WP PR: devgateway/data-viz-wordpress#151

Type of change

• Bug fix (fix:)
• New feature (feat:)
• Breaking change (BREAKING CHANGE:)
• Refactor / chore (refactor: / chore:)
• Documentation update (docs:)

Affected package(s)

• @devgateway/dvz-ui-react
• @devgateway/wp-react-lib
• example only (no changeset needed)

Checklist

• PR title follows Conventional Commits format
• A changeset has been added (pnpm changeset) for any change to a published package
• pnpm build passes locally
• pnpm --filter @devgateway/* typecheck passes
• No hardcoded credentials, internal URLs, client names, or PII introduced
• Any new dependency has an Apache-2.0-compatible license

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@0155471

pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@0155471

commit: 0155471

[Copilot]

🟡 Changes recommended

The current D3MapDataProvider adds production console logging and passes includeColumns outside params (so it won’t reach the API), which should be corrected before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends the embeddable map components (especially d3Map) to support requesting and displaying additional backend columns in tooltip templates, plus adds utilities for flattening API dimension-tree structures into tooltip-friendly variables.

Changes:

• Add per-layer extraTooltipColumns → backend includeColumns support (and guard against requesting columns already used as dimensions to avoid API 500s).
• Enhance tooltip templating to support column names with spaces (via {Column Name}) and add a currency-symbol lookup marker $().
• Add extractVariables/extractVariablesDeep to flatten nested { type, value } nodes into flat tooltip variables, and add row-level color override support via _Color_<measureName>.

File summaries

File	Description
packages/dvz-ui/src/embeddable/tabbedposts/index.jsx	Minor change to category normalization for API params.
packages/dvz-ui/src/embeddable/map/index.jsx	Exclude dimension1/dimension2 from includeColumns to prevent backend 500s.
packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx	Refetch behavior adjustments for non-filter param edits; currently adds debug logging and an ineffective includeColumns param.
packages/dvz-ui/src/embeddable/d3Map/Utils.jsx	Add helpers to flatten dimension-tree rows into variables + resolve row-level override colors.
packages/dvz-ui/src/embeddable/d3Map/Tooltip.jsx	Add space-tolerant variable references and currency symbol marker handling.
packages/dvz-ui/src/embeddable/d3Map/README.md	Document tooltip variable/template behavior, extra tooltip columns, and color override feature.
packages/dvz-ui/src/embeddable/d3Map/Legends.jsx	Ensure legend fetches include extra tooltip columns to avoid store-slot clobbering.
packages/dvz-ui/src/embeddable/d3Map/LatLongLayer.jsx	Include extra tooltip variables by flattening API child nodes into point metadata.
packages/dvz-ui/src/embeddable/d3Map/FlowLayer.jsx	Flatten tooltip variables for flow endpoints and use the configured tooltip template for origin markers.
packages/dvz-ui/src/embeddable/d3Map/DataLayer.jsx	Support row-level _Color_<measureName> override and flatten extra tooltip variables into meta.
.changeset/tame-coats-poke.md	Publish note for the feature set and related fixes.

Review details

Suppressed comments (5)

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:36

• reducers/data.getData only forwards params to the API. Passing includeColumns as a top-level field to loadData() does not affect the request, and can mislead future maintainers. Put includeColumns inside params (preferred at the caller) or remove it here.

            if (!waitForFilters || editing) {
                console.log('📥 [D3 Map DataProvider] Initial data load triggered', { app, editing, waitForFilters });
                this.loadData({ app, source, store, params, group, includeColumns: extraTooltipColumns });
            }

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:44

• Same as above: includeColumns needs to be part of params to reach the API call; passing it as a sibling prop is ignored.

            if (!editing && waitForFilters) {
                this.fallbackTimeout = setTimeout(() => {
                    if (!this.props.data && !this.props.loading) {
                        console.warn('⚠️ [D3 Map DataProvider] Fallback loading triggered');
                        this.loadData({ app, source, store, params, group, includeColumns: extraTooltipColumns });
                    }
                }, 2000);

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:71

• This per-update console.log will spam the console during normal map interaction and can leak data; it should be removed or gated behind a debug flag.

        console.log("Current props in d3MapDataProvider", this.props);

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:115

• This log is triggered on any non-filter prop change and can be very noisy in production; please remove or gate behind a debug flag.

                            console.log('[DataProvider] Props changed, loading data');
                            this.loadData({ app, source, store, params, group });

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:149

• Avoid logging full props during render; it will run frequently and may expose dataset contents in the console.

    render() {
        const { data, } = this.props
        console.log('[DataProvider] Rendering with data:', this.props)

        return <DataContext.Provider value={data}>{this.props.children}</DataContext.Provider>

• Files reviewed: 11/11 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@21b9bcb

pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@21b9bcb

commit: 21b9bcb

[Copilot]

🔵 Needs a closer look

It introduces confirmed runtime-breaking issues (e.g., an undefined extraTooltipColumns reference in D3MapDataProvider) and an inconsistent FlowLayer origin-tooltip variable payload that can break common templates like Value {value}.

Review details

Suppressed comments (8)

Previously missed (2) — in code that hasn't changed since the last review.

packages/dvz-ui/src/embeddable/d3Map/FlowLayer.jsx:235

• The origin marker tooltip now uses the configured tooltip, but the variables passed here omit the normalized {value} used elsewhere (e.g. DataLayer.getTooltipVariables and the flow-line hover path). This can break templates like Value {value} for origins. Consider passing a {value} computed from _value and keeping the same meta merge semantics used in the other flow tooltips.
  packages/dvz-ui/src/embeddable/d3Map/Tooltip.jsx:7
• currencies contains names with spaces (e.g. "USD Dollar", "South Africa Rand"), but this regex does not allow spaces, so $(USD Dollar) / $(South Africa Rand) can never match and the name-based lookup path is effectively dead code.

This issue also appears on line 56 of the same file.

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:34

• extraTooltipColumns is referenced here but not defined in this scope, which will throw a ReferenceError on mount. Also, the data fetch reads includeColumns from params.includeColumns (see reducers/data.js:getData), so this extra top-level field is ineffective—set includeColumns on params (as the layer wrappers already do) and just pass params through.

            if (!waitForFilters || editing) {
                console.log('📥 [D3 Map DataProvider] Initial data load triggered', { app, editing, waitForFilters });
                this.loadData({ app, source, store, params, group, includeColumns: extraTooltipColumns });
            }

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:41

• Same issue as above: extraTooltipColumns is not defined here, causing a ReferenceError in the fallback load path. The fetch should rely on params.includeColumns instead.

                this.fallbackTimeout = setTimeout(() => {
                    if (!this.props.data && !this.props.loading) {
                        console.warn('⚠️ [D3 Map DataProvider] Fallback loading triggered');
                        this.loadData({ app, source, store, params, group, includeColumns: extraTooltipColumns });
                    }

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:145

• Logging this.props on every render can significantly slow down map interactions and floods the console; please remove or guard it behind a debug flag.

        console.log('[DataProvider] Rendering with data:', this.props)

packages/dvz-ui/src/embeddable/d3Map/Tooltip.jsx:60

• Use strict equality when comparing normalized currency identifiers, and avoid recomputing the normalized lookup key three times.

            const currencyNameorCode = result[2]
            const currency = currencies.find(c => processStringForComparison(c.code) == processStringForComparison(currencyNameorCode)
                || processStringForComparison(c.name) == processStringForComparison(currencyNameorCode)
                || processStringForComparison(c.symbol) == processStringForComparison(currencyNameorCode))
            if (currency) {

packages/dvz-ui/src/embeddable/tabbedposts/index.jsx:275

• Array.prototype.join() already returns a string, so the extra .toString() call is redundant and makes this normalization harder to read.

  const normalizedCategories = Array.isArray(categories)
    ? categories.join(',').toString()
    : categories;

packages/dvz-ui/src/embeddable/data/D3MapDataProvider.jsx:69

• This console.log runs on every update and can be very noisy (and expensive in devtools) for a component that updates frequently; it should be removed or gated behind an explicit debug flag.

        console.log("Current props in d3MapDataProvider", this.props);

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite