Skip to content

feat(ui): implement audit v4 phase 4 layer control and legend parity - #41

Merged
dhanyyudi merged 18 commits into
mainfrom
audit-v4-phase4-layer-control-legend
May 2, 2026
Merged

dhanyyudi merged 18 commits into
mainfrom
audit-v4-phase4-layer-control-legend

Conversation

@dhanyyudi

Copy link
Copy Markdown
Owner

@codex please review this Phase 4 layer control and legend styling work.

Summary

  • Add project.layerControlSettings with mode, position, background color, opacity, text color, text size, and radius.
  • Rename layer control mode semantics to qgis2web parity: collapsed, expanded, tree, with legacy compact migrated to collapsed.
  • Apply layer control styling in both editor preview and exported runtime.
  • Fix legend 4-corner placement parity between editor preview and runtime export.
  • Add Phase 4 Playwright parity coverage and update smoke gates for the Phase 4 default collapsed mode.

Verification

  • npm run build: PASS
  • npm run smoke:fixture: PASS
  • npm run smoke:export: PASS
  • npm run smoke:map: PASS, 38 passed
  • npx playwright test tests/map-render.spec.ts -g "phase 4": PASS, 4 passed

Evidence

  • docs/screenshots/phase-4/npm-run-build-20260502-230526.txt
  • docs/screenshots/phase-4/npm-run-smoke-fixture-20260502-230526.txt
  • docs/screenshots/phase-4/npm-run-smoke-export-20260502-230526.txt
  • docs/screenshots/phase-4/npm-run-smoke-map-20260502-230526.txt
  • docs/screenshots/phase-4/npx-playwright-test-tests-map-render-spec-ts-g-phase-4-20260502-230526.txt
  • docs/screenshots/phase-4/editor-20260502-230526.png
  • docs/screenshots/phase-4/editor-legend-bottom-right-20260502-230526.png
  • docs/screenshots/phase-4/editor-legend-top-left-20260502-230526.png
  • docs/screenshots/phase-4/runtime-preview-20260502-230526.png
  • docs/screenshots/phase-4/runtime-20260502-230526.png
  • docs/screenshots/phase-4/console-20260502-230526.txt
  • docs/screenshots/phase-4/network-tile-20260502-230526.json
  • docs/screenshots/phase-4/phase4-export-20260502-230526.zip

Notes

  • Quick review of the manual runtime console entry found one generic 404 line during evidence capture.
  • Tile network evidence did not show a 404 tile request, and all required smoke and Playwright gates passed.
  • Treating the 404 as a non-blocking observation for PR review rather than a Phase 4 gate failure.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 2, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
q2web-studio b3fb84c Commit Preview URL

Branch Preview URL
May 02 2026, 06:47 PM

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b545346e8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/projectHydration.ts Outdated
function migrateLayerControlMode(value: unknown): "collapsed" | "expanded" | "tree" {
if (value === "collapsed" || value === "expanded" || value === "tree") return value;
if (value === "compact") return "collapsed";
return "expanded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Default unknown layer control mode to collapsed in hydration

hydrateProject now forces mapSettings.layerControlMode from layerControlSettings.mode, but migrateLayerControlMode falls back to "expanded" for missing/invalid legacy values. This means older project JSONs that lack both fields are silently upgraded to expanded mode, which contradicts the new Phase 4 default (collapsed) and causes editor/runtime behavior to diverge from expected parity for migrated data.

Useful? React with 👍 / 👎.

@dhanyyudi

Copy link
Copy Markdown
Owner Author

@codex fixed the hydration default mismatch you flagged.

Change made:

  • src/lib/projectHydration.ts now falls back to defaultLayerControlSettings.mode instead of "expanded", so migrated projects without either layer control field now default to collapsed, consistent with Phase 4.

Verification rerun after the fix:

  • npm run build PASS
  • npm run smoke:fixture PASS
  • npm run smoke:export PASS
  • npx playwright test tests/map-render.spec.ts -g "phase 4" PASS

Fix commit:

  • 4b1224f fix(project): default hydrated layer control to collapsed

Please take another look.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b1224fcf8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/projectUpdates.ts Outdated
function normalizeLayerControlMode(mode: LayerControlMode | "compact" | undefined): LayerControlMode {
if (mode === "compact") return "collapsed";
if (mode && phaseCLayerControlModes.includes(mode)) return mode;
return "expanded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Default migrated layer control mode to collapsed

normalizeLayerControlMode still falls back to "expanded" when the incoming project has no mapSettings.layerControlMode, which means legacy projects that lack both layer-control fields are migrated to expanded mode. In this change set, Phase 4 defaults were switched to collapsed, and hydrateProject consumes the migrated value, so these legacy projects will still render/export with the wrong default unless this fallback is updated to collapsed.

Useful? React with 👍 / 👎.

@dhanyyudi

Copy link
Copy Markdown
Owner Author

@codex fixed the follow-up migration fallback you flagged in src/lib/projectUpdates.ts.

Change made:

  • normalizeLayerControlMode() now falls back to defaultLayerControlSettings.mode instead of "expanded", so legacy projects missing both layer-control fields now migrate consistently to collapsed.

Verification rerun after the fix:

  • npm run build PASS
  • npm run smoke:fixture PASS
  • npm run smoke:export PASS
  • npx playwright test tests/map-render.spec.ts -g "phase 4" PASS

Fix commit:

  • 619eebc fix(project): default migrated layer control to collapsed

Please take another look.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 619eebc554

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/runtime/runtime.ts
Comment on lines +200 to +201
var settings = config.layerControlSettings || {};
var mode = settings.mode || legacySettings.layerControlMode || "collapsed";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Persist layer control styling settings to runtime config

applyLayerToggle now depends on config.layerControlSettings, but buildRuntimeConfig (in src/lib/exportProject.ts) still serializes only mapSettings and does not include layerControlSettings. As a result, changes from the new controls (position/background opacity/text size/radius) are stored in project state but dropped from q2ws-config.json, so runtime preview/export silently falls back to defaults (for example top-right with default colors). Please serialize layerControlSettings (or otherwise carry these fields into runtime config) so editor and runtime stay in parity.

Useful? React with 👍 / 👎.

@dhanyyudi

Copy link
Copy Markdown
Owner Author

@codex fixed the runtime config persistence issue you flagged.

Change made:

  • buildRuntimeConfig() in src/lib/exportProject.ts now serializes layerControlSettings into q2ws-config.json.
  • scripts/smoke-export.ts now asserts both default and custom layerControlSettings survive export, including position, background color, opacity, text color, text size, and border radius.

Verification rerun after the fix:

  • npm run build PASS
  • npm run smoke:fixture PASS
  • npm run smoke:export PASS
  • npx playwright test tests/map-render.spec.ts -g "phase 4" PASS

Fix commit:

  • f0a29ee fix(export): persist layer control settings in runtime config

Please take another look.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0a29eec00

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +132 to 133
<label key={layer.id} style={{ color: styleSettings.textColor, fontSize: styleSettings.textSize }}>
<input

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply configured text size to layer label spans

The new "Control text size" setting does not actually change layer-name text in the editor preview because this code applies fontSize to the <label>, while preview CSS still forces .layer-toggle-preview span to font-size: 13px. In practice, any non-default text size will render incorrectly in the editor (while runtime uses the configured size), so users get a misleading preview. Apply the configured size directly to the row text <span> (and tree label spans) or remove the fixed span font-size rule.

Useful? React with 👍 / 👎.

@dhanyyudi

Copy link
Copy Markdown
Owner Author

@codex fixed the editor preview text-size mismatch you flagged.

Change made:

  • src/components/mapCanvasPanels.tsx now applies the configured layer control text size directly to layer-name spans and tree group spans, so the fixed .layer-toggle-preview span CSS no longer overrides the preview text.
  • tests/map-render.spec.ts now sets Control text size to 18px and asserts both editor preview labels and runtime labels render at 18px.

Verification rerun after the fix:

  • npm run build PASS
  • npm run smoke:fixture PASS
  • npm run smoke:export PASS
  • npx playwright test tests/map-render.spec.ts -g "phase 4" PASS

Fix commit:

  • 18957bf fix(ui): apply layer control text size to preview labels

Please take another look.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18957bfb0b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/styles.css
Comment on lines +1124 to 1127
.legend-preview.legend-top-right {
top: 82px;
right: 14px;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Offset editor top-right legend to match runtime layout

When legend placement is top-right and the layer control is also top-right, the editor preview keeps the legend at right: 14px, so it overlaps the control; runtime behavior in this same change offsets that case (#q2ws-layer-control... ~ #q2ws-legend... { right: 248px; }), so editor and runtime no longer match. This breaks the parity this feature is targeting for users configuring floating legend/control positions.

Useful? React with 👍 / 👎.

@dhanyyudi

Copy link
Copy Markdown
Owner Author

@codex fixed the editor top-right legend overlap you flagged.

Change made:

  • src/styles.css now applies the same top-right collision offset in the editor preview: .layer-toggle-preview.layer-toggle-top-right ~ .legend-preview.legend-top-right { right: 248px; }.
  • tests/map-render.spec.ts now asserts the editor preview uses right: 248px when the top-right layer control is present, and falls back to right: 14px when the control is removed, matching the existing runtime assertion.

Verification rerun after the fix:

  • npm run build PASS
  • npm run smoke:fixture PASS
  • npm run smoke:export PASS
  • npx playwright test tests/map-render.spec.ts -g "phase 4" PASS

Fix commit:

  • b3fb84c fix(ui): offset editor top-right legend beside control

Please take another look.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dhanyyudi
dhanyyudi merged commit 03ea1df into main May 2, 2026
2 checks passed
@dhanyyudi
dhanyyudi deleted the audit-v4-phase4-layer-control-legend branch May 2, 2026 18:52
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.

1 participant