Explorer mockup v1: in-map search, sidebar mirror, bottom legend, permanent table (#200)#217
Merged
Merged
Conversation
Relocate the search input, scope buttons (Search Selected Areas / Search Entire World), help text, and results count from the top-of-page .explorer-controls block into an absolutely-positioned .map-search-overlay inside a new .map-wrap div around #cesiumContainer. All element IDs preserved (#sampleSearch, #searchAreaBtn, #searchWorldBtn, #searchResults) so existing handlers in the zoomWatcher OJS cell and the URL state contract continue to work unchanged. Part of isamplesorg#200 mockup-alignment work. The Globe/Table toggle and #tableControls stay in the top-level .explorer-controls block for now; they will be removed in a later commit when the table becomes permanent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add #sampleSearchSidebar input to the top .panel-section of .side-panel. Two-way input-event mirror with the in-map #sampleSearch keeps the two input chrome in sync as a single logical query term. Enter on the sidebar input always submits world scope per the Option B decision in the isamplesorg#200 mockup discussion — a typed-text query from the sidebar implies "find anywhere" rather than "limit to current map view." applyQueryToSearch() now hydrates both inputs from the ?search= URL param. writeQueryState() still reads from #sampleSearch only since the mirror guarantees value parity. The mirror guards against feedback loops by comparing values before setting; programmatic .value assignment does not fire 'input', so a strict-equality guard is sufficient (no debounce or recursion flag). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex flagged three blockers on the cumulative M-1A + M-1B diff: 1. Mobile overlay collided with the Cesium toolbar. At max-width: 900px the overlay was switching to left: 8px, covering the toolbar column at left: 5px. Removed the mobile left-shift; keep the overlay offset at left: 50px so the vertical toolbar remains hit-targetable. Width adjusts to calc(100% - 58px) instead. 2. Base-layer picker dropdown could be occluded by the overlay (dropdown opens at left: 36px; overlay starts at left: 50px). Bump .cesium-baseLayerPicker-dropDown z-index to 1100 so it wins the z-stack against the overlay's 1000. 3. No automated coverage for overlay-vs-toolbar collision. Add tests/playwright/explorer-map-overlay.spec.js with four specs: desktop overlap, mobile overlap, dropdown z-index ordering, and a smoke test that the sidebar↔in-map input mirror is bidirectional. Also picked up the small IME concern from Codex ask #1: gate both Enter handlers (in-map and sidebar) on `!e.isComposing && e.keyCode !== 229` so IMEs that emit Enter to commit a candidate don't trigger a search on the pre-commit value. Non-blocker items (URL-clears-search edge case, aria-label on #sampleSearch, aria-describedby for hints) deferred — to be picked up in the EXPLORER_STATE.md / a11y commit at the end of the PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex round-2 review flagged three remaining items on the M-1A/M-1B overlay work: 1. At 320px (iPhone SE) the two nowrap scope buttons would overflow .search-actions. Add a @media (max-width: 400px) rule that switches .search-actions to flex-direction: column so they stack. 2. The z-index Playwright check was only computed-style math; it would miss an ancestor stacking-context trap or pointer interception. The dropdown spec now adds a real document.elementFromPoint() hit-test at the geometric intersection of the dropdown and overlay rects. 3. Added a dedicated 320px viewport test that asserts both (a) the overlay clears the toolbar and (b) no button overflows its parent row at that width. Also picked up the spec hygiene notes: - test.describe.configure({ timeout: 90000 }) — Cesium/OJS boot is slow on CI; default 30s was too tight. - Mirror-input spec now uses a waitForBootReady() helper that waits for window._ojs before calling value('zoomWatcher'), mirroring the pattern used elsewhere in the suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a horizontal swatch row centered above Cesium's bottom credits row, showing the four source colors with their display names. Mockup requirement is "users can identify what each color means without hunting the sidebar"; functional toggle/show-hide stays in #sourceFilter as the single source of truth. The legend is marked aria-hidden="true" so screen readers don't read the palette twice (the sidebar source filter exposes the same information via labeled checkboxes). It also has pointer-events: none so it never intercepts globe interaction. Hardcoded colors mirror assets/js/source-palette.js. We could fetch SOURCE_COLORS dynamically at render-time, but that pulls a static overlay into the OJS reactive graph for no real benefit — the palette is stable and audited via issue isamplesorg#113. Wraps onto multiple rows below 600px viewport width. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructure the page so the samples table is always visible below the globe rather than hidden behind a Globe/Table toggle button. Removed: - #globeViewBtn / #tableViewBtn buttons and their .view-toolbar / .view-toggle CSS - body.table-view-active class (toggle-mode marker; no longer needed) - isTableViewActive() helper - ?view=table URL param read in writeQueryState() and hydration in the tableView OJS cell - setView() function inside tableView; the unconditional permanent layout replaces it Restructured: - Map height shrinks to clamp(400px, 50vh, 540px) from 500/65/680 so the table fits below without a full-page-height app feel - #tableControls (Max samples input) moved from a top-of-page toolbar into the table section header, next to the new <h3>Samples table</h3> - #tableContainer defaults to display: block (no display: none toggle) Added: - Table row click handler that mirrors the search-row click ceremony (isamplesorg#207 item 8): bump freshness token → set selectedPid → update sample card → flyTo → lazy-load description from wide_url. Same code path as a sample-mode globe-point click, so the URL pid hash, sidebar detail, and selection state stay in sync regardless of which surface the user clicks on. - rowsByPid Map built at refreshTable() time for O(1) row lookup on click — avoids re-scanning a 25K-row array per click. - .samples-table tr.selected styling so the table reflects the currently-selected sample. Selection is repainted in-place on click rather than triggering a full table refresh. The "Max samples" change handler now always refreshes (previously gated on isTableViewActive()). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex flagged three blockers + one cleanup on commit d51e0c5: 1. Playwright layout-stability spec still targeted the removed #tableViewBtn / #globeViewBtn and asserted the pre-M-5 desktop map height range (>=500). Rewrote the desktop assertion path: globe and table now coexist; assert tableContainer.top sits below the globe; assert the toggle buttons have count 0. Updated the desktop clamp bounds (400-540) and the mobile clamp bounds (360-480) to match the new --explorer-map-height values. 2. ?view= was no longer being WRITTEN but was not being canonicalized AWAY either. A bookmarked `?view=table&...` URL would stick through filter/share flows. Added params.delete('view') to writeQueryState() so the legacy param is stripped on the next URL update. 3. Boot-time race: a very-early table-row click could fire before zoomWatcher registered its URL hash listeners, leaving the #pid hash out of sync with viewer._globeState.selectedPid. The table-row click handler now writes the hash directly via history.replaceState(null, '', buildHash(viewer)) so URL canonicality is independent of zoomWatcher init order. Cleanup: - Stale comment "Binary Globe/Table view" → "Samples table (permanent below globe; M-5)". - url-roundtrip.spec.js comment block: removed the stale `view=table` coverage reference; added a one-line note that the param was removed. Plus new test coverage from Codex round-1 ask isamplesorg#7: - tests/playwright/explorer-map-overlay.spec.js: new spec that clicks the first table row, asserts the .selected class repaint, asserts viewer._globeState.selectedPid, and asserts the #pid hash carries the row's pid. Closes the "table-row click as primary selection surface" coverage gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reflect the state-contract changes that landed in this PR: - §1 URL params: ~~view~~ struck (removed in M-5). New row documents search_scope (was already in code, missing from the inventory). - §3 DOM-as-state: ~~body.table-view-active~~ struck. New rows added for the permanent samples table's data-pid attribute on <tr data-pid> and the tr.selected visual marker. - §3 input-elements paragraph: #sampleSearchSidebar listed alongside #sampleSearch with a note that the two are mirrored via input-event handlers. - §5 OJS cell graph: tableView no longer writes ?view to URL; it now writes the #pid hash via buildHash from the table-row click handler (mirrors sample-mode globe click). - §7 facet-count contract: "view mode" row struck — moot since the Globe/Table toggle is gone. - §6 new "Mockup-v1 addendum": the canonical writeup of M-1A, M-1B, M-2, M-5 — what changed, why, what stayed (data contract is unchanged from option C + the isamplesorg#178 light path). - Top-of-doc note flagging the inventory has had targeted edits since the original 94e7674 baseline; line citations may not match exactly. This is documentation only — no code changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex flagged four sets of doc errors in c552351: 1. Stale view-mode references remained: - §1 search row: "called from doSearch() and from globe/table toggle" → removed the toggle reference (and the toggle is gone). - §4 hydration paragraph: "tableView ⇒ view hydration" → updated to note the hydration step was removed in mockup-v1. - §5 cell graph: tableView "reads view from URL once" → updated to "calls refreshTable() unconditionally on boot — no URL hydration". - §5 "two cells register change listeners" note: removed the "marks dirty and refreshes only if visible" half — tableView now refreshes unconditionally. 2. Table-row click ceremony writeup tightened: - Added explicit preconditions (anchor-element bail; missing sample/lat-lng bail; typeof viewer === 'undefined' bail). - Moved the .selected repaint into the ceremony list as step 6, synchronous, before the async detail query. - Step 7 now mentions the pid SQL-escaping via .replace(/'/g, "''"). - Reworded "same five-step ceremony" → "same async-selection ceremony with one table-specific addition", since the direct history.replaceState(buildHash) is unique to table-row click. 3. ?view= canonicalization wording narrowed: - Added caveat that only the next writeQueryState() call strips the param; hash-only writes via buildHash() preserve location.search as-is. 4. §5 zoomWatcher event-listener cell: added the sidebar #sampleSearchSidebar input/keydown listeners to the list, plus the in-map keydown that was already noted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex round-2 caught a contradiction: the zoomWatcher row claimed Share writes through writeQueryState(), but explorer.qmd:2477 shows Share actually uses history.replaceState(null, '', buildHash(viewer)), which is a hash-only write that preserves location.search. The new mockup-v1 addendum caveat already lists Share correctly as a buildHash caller; this commit aligns §5 with that reality. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gap) Codex round-3 spotted a pre-existing doc gap: zoomWatcher registers both viewer.camera.changed AND viewer.camera.moveEnd (added in isamplesorg#205 for sub-threshold pan settling), but §5 listed only camera.changed. Adding moveEnd to both the event-listeners and the URL-writes cells. Not strictly mockup-v1 scope but landing while we're here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes most of #200 mockup-alignment scope. Sequenced into independently revertable commits.
Summary
.map-search-overlayover#cesiumContainer. All element IDs preserved.#sampleSearchSidebarinput mirrored to#sampleSearchvia two-wayinput-event handlers. Enter on sidebar always submits world-scope (Option B)..map-color-legendwith the four source-palette swatches.aria-hidden,pointer-events: none— does not duplicate the filter source-of-truth in#sourceFilter.#globeViewBtn,#tableViewBtn,body.table-view-active,?view=URL param) removed. Table row click reuses the sample-mode globe-click selection ceremony — setsviewer._globeState.selectedPid, updates the sidebar, flies the camera, writes#pidto the hash directly so the URL is canonical regardless ofzoomWatcherinit order.EXPLORER_STATE.mdupdated with a new §6 "Mockup-v1 addendum" plus targeted strikes for the removed state surfaces. Also picks up a pre-existing doc gap (missingcamera.moveEndlistener in §5).Codex review trail
Each segment was iterated via
codex execuntil Codex returnedLGTM, ready to merge:elementFromPointclick-through testquarto render)?view=not canonicalized;#pidhash race beforezoomWatcherwirescamera.moveEndlistenerState-contract delta (full writeup in
EXPLORER_STATE.md§6 mockup-v1 addendum)?view=URL param,body.table-view-activeclass,isTableViewActive(),setView(),#globeViewBtn/#tableViewBtn.writeQueryState()now doesparams.delete('view')to canonicalize legacy bookmarks (nextwriteQueryState()call only — hash-only writes preservelocation.search).#sampleSearchSidebarinput mirrored to#sampleSearch..map-search-overlay,.map-color-legend,.map-wrapDOM surfaces.tr.selectedvisual marker on.samples-table tbody tr[data-pid].rowsByPid: Mapcached atrefreshTable()time for O(1) per-PID lookup..selectedclass. The table only repaints.selectedon nextrenderTable(). Bidirectional highlight is a follow-up.Test plan
explorer-layout-stability.spec.jsupdated for new map height clamp (400-540 desktop, 360-480 mobile) and the now-permanent tableexplorer-map-overlay.spec.js: overlay vs Cesium toolbar (desktop, 390px, 320px); base-layer dropdown click-through viaelementFromPoint; sidebar↔in-map input mirror; table-row click →.selectedclass +viewer._globeState.selectedPid+#pidhashurl-roundtrip.spec.jsdoc-comment updated; no spec deletions neededDeferred (non-blocking)
aria-labelon#sampleSearch,aria-describedbylinking the help/hint textapplyQueryToSearch()does not clear inputs when URL drops?search=(back/forward navigation edge case).selectedclass repaint when sample selected from globe or search)🤖 Generated with Claude Code