Phase 2 (6/6): fix district search tooltip bug; skip touch-tap here deliberately - #462
Open
JeffreyBLewis wants to merge 1 commit into
Open
Phase 2 (6/6): fix district search tooltip bug; skip touch-tap here deliberately#462JeffreyBLewis wants to merge 1 commit into
JeffreyBLewis wants to merge 1 commit into
Conversation
…rt (Phase 2, site 6 of 6) Last of the 6 tooltip sites from #449, and structurally different from the other 5: a plain jQuery mouseover/mouseout on each search-result table row (not an SVG chart), showing the member's ideology score. Found a real, currently-live bug while looking at this one: the handler was `tr.on("mouseover", () => { ... $(this) ... })` -- an arrow function, which doesn't rebind `this` the way a normal function does. `this` inside resolved to the enclosing scope, not the row, so `$(this).children(...) .offset()` returned undefined and `.offset().left` threw a TypeError on every hover. The tooltip has never actually shown up. Confirmed by running the exact code (via a temporary local harness loading district.js directly against real /api/districtLookup data, removed after testing) before and after the fix. Fixed by using `tr`, already the correct row reference in scope, instead of `$(this)`; also dropped a leftover debug console.log that had the same bug and would have thrown first regardless. Considered, then deliberately skipped, adding touch-tap support here: each row already has its own click handler that navigates to the member's full bio page (same ideology score, fuller context). Testing confirmed a tap on touch navigates immediately -- so a competing tap-to- show-tooltip binding would show the tooltip for a moment and then get superseded by the navigation anyway, adding real complexity (the same one-dismiss-listener-for-N-rows split used in party.js/committee.js, needed here too since there can be 100+ result rows) for no actual benefit. Left it hover-only, matching mouse behavior; touch users reach the same information via the tap-to-navigate they already have. Verified with headless Chromium: - Desktop (mouse): hover now correctly shows the tooltip with content and hides on mouseout (previously threw and never appeared) - Touch: confirmed tapping a row still navigates to the member's page (unaffected -- nothing here was touching that handler) - Full existing 41-check suite (unrelated files, sanity check only) still passes This completes Phase 2 (#449) -- all 6 tooltip sites across the 5 listed files (vote map, party map + ideology chart, committee ideology chart, person ideology bucket, district search results) now handle touch appropriately, whether that means adding tap support or, as here, recognizing it wouldn't add anything. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT
6 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 #461 so this diff only shows the new commit, `361cfa7`. This completes Phase 2 (#449).
Last of the 6 tooltip sites, and structurally different: a plain jQuery mouseover/mouseout on each district-search-result table row (not an SVG chart).
Found a real, currently-live bug: the handler was an arrow function using `$(this)`, which doesn't rebind `this` -- it resolved to the enclosing scope, not the row, so `.offset()` returned `undefined` and threw on every hover. The tooltip has never actually worked. Confirmed before/after with a temporary local test harness (loaded `district.js` directly against real `/api/districtLookup` data, removed after testing -- never part of any commit). Fixed by using `tr`, already the correct row reference in scope.
Deliberately did not add touch-tap support here. Each row already has its own click handler that navigates to the member's full bio page. Tested and confirmed: a tap on touch navigates immediately, so a competing tap-to-show-tooltip binding would flash the tooltip and then get superseded by navigation anyway -- real complexity (the same one-dismiss-listener-for-N-rows split as party.js/committee.js, since there can be 100+ rows) for no benefit. Left it hover-only; touch users reach the same info via the navigation they already have.
Test plan
Verified with headless Chromium:
🤖 Generated with Claude Code
https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT