Phase 2 (2-3/5): touch support for the party page's map + ideology chart tooltips - #459
Open
JeffreyBLewis wants to merge 1 commit into
Open
Phase 2 (2-3/5): touch support for the party page's map + ideology chart tooltips#459JeffreyBLewis wants to merge 1 commit into
JeffreyBLewis wants to merge 1 commit into
Conversation
…of 5) Same treatment as the vote map (#458), applied to party.js's two tooltip sites: 1. The party map's district tooltip -- structurally identical to the vote map's, refactored the same way (named show/hide/position functions, addTouchTooltip on the shared path selection). 2. The ideology chart's per-point tooltip -- more involved, since it binds an individual mouseover/mouseout/mousemove handler per data point (d3.select(obj).on(...) inside a per-node closure, not one shared selection), for potentially hundreds of points across all the party-median lines. Calling the same addTouchTooltip once per point would've added one document-wide dismiss listener per point. Split the touch-tooltip helper into two pieces to avoid that: bindTouchTap (per element, adds the tap-to-show binding and drops the unreliable hover ones) and bindTouchDismiss (called once for the whole set of points, adds a single shared dismiss listener). addTouchTooltip from the map fix is now just these two called together, for sites that only need one selection. The show/hide/position functions for the ideology tooltip didn't need per-point closure state beyond what showFn already captured, so they're defined once and reused across every point instead of being redefined in the loop. Verified with headless Chromium (Playwright) on /parties/100: - Desktop (mouse): hover still works for both the map and individual ideology-chart points, showing correct content and hiding on mouseout - Touch (iPhone 13 emulation): tapping a state on the map, and tapping an ideology-chart point, both show the tooltip with correct content; tapping elsewhere dismisses either; no page errors - Full existing 36-check suite (unrelated files, sanity check only) still passes Found but not fixed here, flagging separately: this page's own charts (#dim-chart, #party-map-chart, #time-chart) don't have the responsive container sizing that the vote/congress pages' scatter plot and map got -- their unconstrained width actually inflates the mobile layout viewport in testing (1175x2001 instead of 390x844), the same class of "too wide for mobile" issue Phase 1 addressed elsewhere, just not extended to this page. Separate task from touch tooltips. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT
4 tasks
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.
Summary
Stacks on #458 so this diff only shows the new commit, `8ed8d48`.
Same treatment as the vote map, applied to `party.js`'s two tooltip sites:
Split the helper into `bindTouchTap` (per element, adds tap-to-show and drops the unreliable hover bindings) and `bindTouchDismiss` (called once for the whole set of points). `addTouchTooltip` is now just these two called together, for sites needing only one selection.
Test plan
Verified with headless Chromium on /parties/100:
Found but not fixed here, flagging separately: this page's own charts (`#dim-chart`, `#party-map-chart`, `#time-chart`) don't have the responsive container sizing that the vote/congress pages' scatter plot and map got in #457 -- their unconstrained width actually inflates the mobile layout viewport in testing (1175x2001 instead of 390x844). Same class of "too wide for mobile" issue Phase 1 addressed elsewhere, just not extended to this page yet. Separate task from touch tooltips, not addressed in this PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT