fix: code review findings#13
Merged
Merged
Conversation
- BarChart/ColumnChart: sortedData sorted itself instead of loadedData,
so enabling sortBy rendered an empty chart; sort loadedData instead
- ColumnChart: fall back to 0 when d3.max is undefined so a missing/
non-numeric value field no longer yields a NaN y-scale (vanished bars)
- StackedColumnChart: use ?? so an explicit maxValue of 0 is honoured
- StackedBarChart: stop throwing in barStyle on a zero total; log and
fall back to 100 like StackedColumnChart
- Bar/Column/LineChart: clamp padded width/height (and Bar range,
StackedColumn tickSize) to >= 0 to avoid negative SVG dimensions
- BarChart: remove dead initialize() axis builder and its watcher
- useChart: emit('loaded', loadedData) and detect highlights on
loadedData so URL-fetched data works
- add sortBy regression tests for BarChart and ColumnChart
- SymbolMap: build the tooltip target id from the marker cursor value directly instead of re-extracting it through markerId(), which yielded "...-marker-undefined" and never anchored the tooltip - ChoroplethMap: drop the root @click="draw" that rebuilt the whole SVG on every click and undid the feature click-to-zoom - ChoroplethMap: stop double-binding mapZoom and binding both planar and spherical zoom behaviors at once; route programmatic zoom through the behavior actually bound to the selection - SymbolMap: replace per-marker categories.indexOf (O(n^2)) with a memoized name->index Map
- SharingOptions: fix malformed selector 'meta[name="description]' (missing quote) that threw a SyntaxError in real browsers, and stop Object.assign mutating the metaValues computed cache - FormControlSelectableDropdown: match the active item by value so keyboard navigation works for object items (the spread clones broke reference-based indexOf) - FormEmbed: build the iframe src from the resolved url argument instead of the possibly-null props.url - FormAdvancedLink: import the real AdvancedLinkTab type instead of the non-existent Tab; FormAdvancedLinkTab: drop the invalid id default - FormDonate: drop the no-op self-assignment in the amount watcher - PaginationTiny: clamp submitted page/row input to [1, numberOfPages]
- HapticCopy: clear the pending tooltip timeout on unmount - SlideUpDown: remove the transitionend listener on unmount - ActiveTextTruncate: stop the RAF loop and remove transition listeners on unmount - ResponsiveIframe: remove the pym Parent (and its window listener) on unmount - SharingOptionsLink: clear the polling interval / popup on unmount - useResizeObserver: guard the element deref, disconnect the observer on unmount, and debounce resize so consumers redraw once dimensions settle - useQueryObserver: disconnect MutationObservers on scope dispose - EllipsisTooltip: disconnect the ResizeObserver on unmount - assets: reject (and uncache) failed asset loads and resolve injectAssets() immediately for an empty list, so callers never hang
- FormControlSelectableDropdown: strip recycle_scroller_id from BOTH sides in firstActiveItemIndex so keyboard nav works after a click or range-select (activeItems then holds items_ entries that carry the key) - assets: count a failed injection as settled (.catch(allFilesLoaded)) so one failed asset no longer leaves injectAssets pending forever - ChoroplethMap: route resetZoom and setFeatureZoom through a shared activeZoomBehavior too, so spherical maps no longer dispatch those zooms via the planar handler - useResizeObserver: cancel the debounced callback on unmount (and drop the now-redundant unobserve) so no trailing call fires after teardown - useChart: dataHasHighlights falls back to the raw data prop so a synchronous array reports highlights on first paint - FormAdvancedLink tests: import AdvancedLinkTab instead of the non-existent Tab type
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.
Routine cleaning and fixes with /code-review, no external review needed.