Skip to content

chore: enable multiple measures to be selected in a map - #150

Merged
ralvarez-dg merged 4 commits into
mainfrom
task/enable-multiple-measures-to-be-selected-in-a-map
Sep 3, 2026
Merged

chore: enable multiple measures to be selected in a map#150
ralvarez-dg merged 4 commits into
mainfrom
task/enable-multiple-measures-to-be-selected-in-a-map

Conversation

@timothygachengo

Copy link
Copy Markdown
Contributor

Description

Re-enable Multiple measures to be selected in a map.

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-wp-commons
  • @devgateway/create-wp-customizer
  • @devgateway/upgrade-wp-customizer
  • plugins/wp-react-blocks-plugin
  • plugins/wp-react-custom-api
  • plugins/wp-react-custom-rest-menu
  • Other plugin / theme / Docker (no changeset needed)

Checklist

  • PR title follows Conventional Commits format
  • pnpm build passes locally
  • No hardcoded credentials, internal URLs, client names, or PII introduced
  • Any new dependency has a GPL-2.0-or-later-compatible license (MIT, BSD, Apache-2.0, ISC are all compatible)

Copilot AI lite review requested due to automatic review settings September 3, 2026 08:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new app prop handling and componentDidUpdate type dereferencing can throw at runtime and should be corrected before approval.

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

Pull request overview

Re-enables multi-measure selection behavior for the Map block’s measure selector UI, while also adjusting map type handling during block updates to better tolerate missing/empty type lists.

Changes:

  • Adds a supportsMultipleMeasures switch to render multi-select toggles vs single-select behavior in the Measures panel.
  • Refines currentType selection logic in APIConfig.componentDidUpdate to avoid failing when types is empty.
  • Adds React key usage for measure rows in grouped rendering.
File summaries
File Description
plugins/wp-react-blocks-plugin/blocks/map/Measures.jsx Updates measures UI to conditionally support multi-select and improves list rendering.
plugins/wp-react-blocks-plugin/blocks/map/APIConfig.js Adjusts type lookup logic during updates to be more defensive when resolving the current type.
Review details

Suppressed comments (1)

plugins/wp-react-blocks-plugin/blocks/map/Measures.jsx:63

  • The grouped <PanelBody> elements are rendered from an array but don’t have a key prop, which will trigger React key warnings and can lead to incorrect reconciliation when groups change.
            [...new Set(allMeasures.map(p => getTranslation(p.group)))].map(g => {
                    return (<PanelBody title={g}>
                            {allMeasures.filter(f => getTranslation(f.group) === g).map(m => <PanelRow key={m.value}>
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

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

Comment thread plugins/wp-react-blocks-plugin/blocks/map/APIConfig.js Outdated
Comment thread plugins/wp-react-blocks-plugin/blocks/map/Measures.jsx
Copilot AI review requested due to automatic review settings September 3, 2026 08:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are confirmed runtime-crash paths in APIConfig.componentDidUpdate due to unguarded types.filter(...) and dereferencing a potentially-null prevTypeObject.

Review details

Suppressed comments (3)

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

plugins/wp-react-blocks-plugin/blocks/map/Measures.jsx:66

  • The grouped measures map(g => ...) returns a <PanelBody> without a key, which will trigger React key warnings and can cause unstable panel rendering when groups change.

plugins/wp-react-blocks-plugin/blocks/map/APIConfig.js:142

  • types is now guarded for currentType, but prevTypeObject still calls types.filter(...) unconditionally. If types is ever undefined/empty (which the new guard suggests can happen), this will throw before the guard is reached.
        const prevTypeObject = types.filter(t => t.value === prevType).length > 0 ? types.filter(t => t.value === prevType)[0] : null
        const currentType = types && types.length > 0
            ? types.find(t => t.value === type)
            : null

plugins/wp-react-blocks-plugin/blocks/map/APIConfig.js:145

  • prevTypeObject can be null (e.g., previous type no longer present in types), but this dereferences prevTypeObject.supports unconditionally and will throw.
            if (prevTypeObject.supports.singleMeasure != currentType.supports.singleMeasure || (currentType.supports.singleMeasure == false && dimension2 != "none")) {
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 3, 2026 08:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are confirmed runtime crash risks (null/undefined access) and a missing React list key in modified code paths that should be fixed before approval.

Review details

Suppressed comments (2)

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

plugins/wp-react-blocks-plugin/blocks/map/Measures.jsx:67

  • The elements generated from the groups map are missing a React key prop, which will produce list rendering warnings and can cause unstable UI updates in the editor.

plugins/wp-react-blocks-plugin/blocks/map/APIConfig.js:147

  • componentDidUpdate still has null/undefined hazards: prevTypeObject uses types.filter(...) without guarding types, and if (prevTypeObject.supports) will throw when prevTypeObject is null. This can crash the block when types is missing (e.g., older saved blocks) or when the previous type isn't present in the types list.
        const prevTypeObject = types.filter(t => t.value === prevType).length > 0 ? types.filter(t => t.value === prevType)[0] : null
        const currentType = types && types.length > 0
            ? types.find(t => t.value === type)
            : null

        if (type != prevType && currentType) {
            if (prevTypeObject.supports) {
                if (prevTypeObject.supports.singleMeasure != currentType.supports.singleMeasure || (currentType.supports.singleMeasure == false && dimension2 != "none")) {
                    setAttributes({measures: [], filters: []})
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 3, 2026 09:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are confirmed correctness issues (missing React list key and a type-change path that can leave stale filters/measures) that should be addressed before approval.

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

Review details

Suppressed comments (1)

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

plugins/wp-react-blocks-plugin/blocks/map/Measures.jsx:66

  • The group created inside the map() loop is missing a React key, which can cause unstable rendering and console warnings when groups change.
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread plugins/wp-react-blocks-plugin/blocks/map/APIConfig.js
@ralvarez-dg
ralvarez-dg merged commit 38f1b9b into main Sep 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants