Scale the DW-NOMINATE scatter plot and vote/district map to fit their container - #457
Scale the DW-NOMINATE scatter plot and vote/district map to fit their container#457JeffreyBLewis wants to merge 4 commits into
Conversation
…flowing Retry of the approach from the earlier, reverted #454: give every DC.js chart's SVG a viewBox matching its native pixel size, then let CSS (.dc-chart > svg { width:100%; height:auto }) shrink the box to fit its container -- the chart's internal coordinate system is unchanged, so D3's mouse/brush math (which resolves screen coordinates through the SVG's own transform) keeps working at any scale. What actually broke last time: this bundled dc.js predates per-chart renderlet callbacks. dc.renderlet(fn) is a page-global hook -- dc.renderAll() and dc.redrawAll() invoke it as dc._renderlet(group), passing the chart-group name (usually undefined), not a chart instance. The previous attempt treated that argument as a chart and called `chart.svg`, throwing a TypeError on every single render/redraw call, which silently aborted whatever ran right after dc.renderAll() (decorateNominate(), which draws the scatter plot's axes/shading) and left every chart's SVG without a viewBox at all, so the new width:100% CSS just clipped it instead of scaling it. Fixed by reading the actual chart list via dc.chartRegistry.list(group) -- the same lookup dc.renderAll/redrawAll use internally -- instead of misreading the argument as a chart. Applied to all six chart-init files (congress.js, voteCharts.js, party.js, committee.js, personIdeology.js, partyGlance.js) since dc.renderlet is a single global hook, not per-page. Also carries over the container-level responsive fixes from the original #454 (scatter/map container sizing, #memberList, #memberTextList, .loadVotes, tooltip width, #geoMap #map-chart made block instead of inline-block to avoid an ambiguous width:100%-in-shrink-to-fit case), and wraps #voteList in a scrollable .voteListScroll container. And keeps the mapPanZoom.js dimX=850 fix (was 890, didn't match mapChart's own width(850) -- would show as a jump on first zoom now that the map gets a real baseline viewBox on load instead of none at all). This time verified against the actual running app with headless Chromium (Playwright) rather than a syntax check and HTTP status codes -- 30 checks across congress/vote/party/person pages at both 390px and 1280px: - No console or page errors on load, and none introduced by interacting with the charts (clicking to brush-select on the scatter plot, clicking a map district to filter) - decorateNominate's output (axes, yea/nay cutline labels on an actual vote, shaded heatmap) is present after the fix -- this is exactly what silently broke last time - Map districts have real, finite geometry (not clipped to nothing) - Both SVGs' rendered width fits their container at both viewport widths - Click-to-brush-select and click-a-district-to-filter both still produce the expected selection filter bar - Resizing the viewport mid-session doesn't break the chart - The sticky vote-count chart (#455) and single-column vote list (#456) fixes from the last two PRs are still intact Also found, but did NOT fix (pre-existing on unmodified master, unrelated to this change, confirmed by reproducing it before touching any code): clicking a map district to filter the vote logs a console error from nominateHeatMap.js's cutline-arc path string construction ("Expected arc flag") for some filtered subsets -- looks like a NaN produced by a division edge case in the cutline math for lopsided/small subsets. Separate bug, separate fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT
Both reported after the last push, both real: 1. Map pushed right with its eastern edge (Maine) clipped. The previous commit hardcoded the map's viewBox to "0 0 850 500" on the assumption it would match mapChart.width(850).height(500) exactly. It doesn't -- the actual rendered path geometry spans roughly x:31-866, y:11-497, so content past x=850 was clipped while the unused 0-31 margin on the left showed as blank space. Fixed by computing the viewBox from the real union of all rendered path bounding boxes (setMapContentViewBox in voteCharts.js) instead of guessing from the configured width/height. Also reverted the mapPanZoom.js dimX 890->850 change from the previous commit -- it was based on the same flawed "should match width()" assumption, and since neither value actually matches the true content bounds, reverting to the original avoids changing pan/zoom behavior that isn't part of today's fix. 2. The vote-count sidebar bar chart (#party-chart) got noticeably smaller text on desktop. Root cause: the previous commit's dc.renderlet hook was left registered globally in voteCharts.js even after switching to the targeted setScatterViewBox/setMapContentViewBox calls, so it was still adding a viewBox to *every* dc.js chart on the page, including #party-chart. That chart's native width (280px) is wider than its sidebar column (~195px), which was never a problem before -- it just rendered at native size, slightly overflowing unnoticed. Once it had a viewBox, the width:100% CSS scaled the whole thing (text included) down to fit the narrow column. Removed the leftover global hook entirely, along with the generic .dc-chart CSS scaling rule; only #scatter-chart and #map-chart get this treatment now; the other four chart-init files (party.js, committee.js, personIdeology.js, partyGlance.js) are back to their pre-mobile-work state. Verified with headless Chromium against the actual running app (34 checks, up from 30): all previous checks still pass, plus new checks confirming the map's rendered content stays fully inside its viewBox at both viewport widths with only a few pixels of intentional padding, and that #party-chart is no longer scaled (no viewBox, renders at its native 280px regardless of viewport). Visually confirmed via screenshots at 1280px and 390px -- Maine fully visible, no left margin, bar chart legend text back to its original size. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT
|
Update: both issues reported were real regressions from over-broad scoping in the first commit -- root-caused and fixed in dc765bd.
Re-verified with headless Chromium: 34 checks (up from 30), all passing, including new checks specifically for these two regressions (map content fully inside its viewBox with clean padding; #party-chart has no viewBox and renders at native width). Screenshots at 1280px and 390px confirm visually -- Maine fully visible, no left margin, bar chart legend text back to its original size. |
…xed height #scatter-container had a fixed height:425px, sized for the chart at its native 890px width. Once the chart's SVG started scaling its width down to fit narrow phone screens (this branch's earlier commits), its rendered height shrank proportionally too -- but the container's height didn't, leaving a large gap between the bottom of the now-much-shorter chart and whatever follows it on the page (the explanatory text block on the vote page). Fixed by setting the container's aspect-ratio from the chart's actual width()/height() once they're known (setScatterViewBox in decorate.js), so its height always tracks its rendered width. aspect-ratio only takes effect when height isn't otherwise fixed, so height has to be relaxed to auto at the same time -- base.css keeps the old fixed height only as a pre-JS fallback. Verified with headless Chromium: the gap between the chart and the following text is 10px at both 390px and 1280px (was ~260px at 390px before this fix, computed from the container's old fixed 425px height vs. the chart's actual scaled-down rendered height). Re-ran the full 36-check suite from the last two commits on this branch -- all still pass, confirming this doesn't disturb the scatter/map scaling, click interactions, or the sticky/column fixes from the earlier PRs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT
|
Another fix added (0ba45db): the large gap between the bottom of the scatter plot oval and the explanatory text block below it on narrow phone widths. Cause: `#scatter-container` had a fixed `height:425px`, sized for the chart at its native 890px width. Once the chart's SVG started scaling its width down to fit narrow screens, its rendered height shrank proportionally too (aspect ratio is fixed), but the container's height stayed at the old fixed 425px, leaving a large empty gap. Fix: set the container's `aspect-ratio` from the chart's actual width()/height() once known, so its height always tracks its rendered width instead of a stale fixed value. Verified: gap between chart and following text is 10px at both 390px and 1280px (was ~260px at 390px before this). Full 36-check suite still passes. |
… pattern base.tpl already busts its CSS cache on every request (cache_breaker = random int, appended as ?t=... to base.css/dc.css/every extra_css entry), but that variable only exists in base.tpl's own template scope -- Bottle renders a rebase()'d child template like congress.tpl or vote.tpl separately, before splicing its output into base.tpl, so the two don't share template-local variables. The app-level <script> tags each of those templates writes directly (decorate.js, congress.js, voteCharts.js, etc.) had no cache-busting at all, unlike the CSS. This isn't just a nice-to-have: it caused real confusion earlier in this branch's work -- a JS fix could be live on the server while a browser that had already loaded the page kept running the old cached copy, making a real fix look like it hadn't taken effect. Give each of these two templates its own cache_breaker (same random.randint(10000, 99999) technique as base.tpl, just computed locally since the two scopes aren't shared) and append it to every non-vendor <script src> tag -- vendor libs under js/libs/ are left alone since they don't change during this work, matching how extra_css already only busts the app's own stylesheets. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT
|
Confirmed stable now -- 45/45 requests clean across congress and vote pages after a direct reload, and the full 36-check verification suite passes. Root cause of the intermittent 500s was stale per-worker template caching (Bottle compiles/caches `.tpl` files in memory per-process; a worker that compiled a momentarily-broken intermediate edit kept erroring until it restarted) combined with `touch-reload` not reaching this specific uwsgi instance. Also pushed ae5f1c0: added cache-busting to congress.tpl/vote.tpl's own `<script>` tags, matching the pattern base.tpl already uses for CSS. Their JS previously had no cache-busting at all, which is exactly the kind of thing that made this debugging session confusing (a fix live on the server, but a browser still running an old cached copy). Should prevent this specific flavor of "is my fix even live" confusion going forward. |
1 participant
Summary
Stacks on #456 (which stacks on #455) so this diff only shows the new commit,
f14099e.Retry of the approach from #454 (reverted after it broke the site). Give every DC.js chart's SVG a
viewBoxmatching its native pixel size, then let CSS (.dc-chart > svg { width:100%; height:auto }) shrink the box to fit its container -- the chart's internal coordinate system doesn't change, so D3's mouse/brush math (which resolves screen coordinates through the SVG's own transform) keeps working at any scale.What broke last time, and the actual fix
The bundled
dc.jshere predates per-chart renderlet callbacks:dc.renderlet(fn)is a page-global hook, anddc.renderAll()/dc.redrawAll()invoke it asdc._renderlet(group)-- passing the chart-group name (usuallyundefined), not a chart instance. The previous attempt didchart.svgon that value, throwing aTypeErroron every render/redraw call, which silently aborteddecorateNominate()(draws the scatter plot's axes/shading) and left every chart without aviewBoxat all -- so the CSS just clipped instead of scaling.Fixed by reading the actual chart list via
dc.chartRegistry.list(group), the same lookupdc.renderAll/dc.redrawAlluse internally. Applied to all six chart-init files since the hook is global, not per-page:congress.js,voteCharts.js,party.js,committee.js,personIdeology.js,partyGlance.js.Also carries the container-level CSS from #454 (
#scatter-container/#memberList/#memberTextList/.loadVotes/tooltip sizing,#geoMap #map-chartasdisplay:blockinstead ofinline-blockto avoid an ambiguous width:100%-in-shrink-to-fit case), wraps#voteListin a scrollable container, and keeps themapPanZoom.jsdimXfix (890 -> 850, to matchmapChart.width(850)).Test plan
This time verified against the actual running app with headless Chromium (Playwright), not just a syntax check and HTTP status codes -- 30 checks across congress/vote/party/person pages at both 390px and 1280px, all passing:
decorateNominate's output (axes, yea/nay cutline labels on a real vote, shaded heatmap) is present after the fix -- this is exactly what silently broke last timeFound, not fixed (pre-existing, unrelated): clicking a map district to filter the vote logs a console error from
nominateHeatMap.js's cutline-arc path string construction ("Expected arc flag") for some filtered subsets. Confirmed present on unmodified master before touching any code, so not a regression from this change -- looks like a NaN from a division edge case in the cutline math for lopsided/small subsets. Flagging as a separate, pre-existing bug rather than fixing it here.Screenshots (390px, before -> after):
🤖 Generated with Claude Code
https://claude.ai/code/session_013KJMYfgTNsxrmjHZpZKAHT