Make FilmDrop UI into a component - #579
Open
Xenocide122 wants to merge 49 commits into
Open
Conversation
…ernals verification, & release-workflow hardening Co-authored-by: Copilot <copilot@github.com>
…seUrlStateSync deps fix, starter example, +14 tests Co-authored-by: Copilot <copilot@github.com>
…rter/types verify scripts; fix EnhancedDetails CSS ownership so it imports its own stylesheet instead of relying on PopupResults. Co-authored-by: Copilot <copilot@github.com>
…types, starter workspace tests), promote audit-prod to blocking, and add least-privilege permissions and concurrency cancellation.
Co-authored-by: Copilot <copilot@github.com>
Tightens hook and context patterns to remove render thrash and stale state risks: - TextField: replace prop-sync useEffect with render-phase setState. - NumericRangeInputs: consolidate twin effects into one. - PopupResults: consolidate dispatching effects; track stable deps. - EnhancedDetailsDisplay: dedupe error alerts via lastErrorKeyRef. - LinkItem, PopupResult, OverflowTooltip: add isMountedRef guards. - LayoutContext, EnhancedDetailsContext: memoize provider values. - useUrlInitialize, useUrlStateSync: rename refs to *Ref convention. - useResizablePanel: stabilize updateColumns via ref-mirrored flag. - App.jsx, LeftContent: correct effect dep arrays. Adds tests: TextField (3), LayoutContext memo, NumericRangeInputs no-clobber, PopupResults dispatch-count.
Wrap each returned function in useCallback and the returned object in useMemo so consumers can list these callbacks in useEffect deps without causing re-runs every render. Adds two reference-identity tests: stability across rerenders with unchanged params, and invalidation of collectionId-dependent callbacks when the collection changes.
Mirrors the LayoutContext memoization test: a memoized probe consumer verifies that re-rendering the provider with an unrelated prop change does not re-render consumers that read context, locking in the useMemo'd provider value contract.
When children transitions from non-empty to empty, the early-return guard previously left isOverflowing at its prior value. Reset it to false so a stale true cannot persist; React's primitive-setState bailout avoids any extra render when the value is already false.
The 'rapid result changes' test now asserts an exact count (2 — one dispatch per results-identity change across two rerenders) instead of the previous loose <= 2 bound. Adds a comment near the consolidated effect explaining why _currentPopupResult is in deps despite being written by the effect (the includes-guard plus Immer's identity-bail prevent the apparent loop).
Both fields are slice-local: dateTime has no reducer/dispatch and showVisualizationList has a reducer that is never dispatched and a state field that is never read. No behavior change.
The action was dispatched on collection change in CollectionDropdown but the resulting state was never read by any production code path. The only consumer was a test asserting on the dispatch side effect. Removing the state, action, dispatch, and the tautological test.
Dispatched once in clearSearch but never read by any component or selector. Removed state, action, export, the import in searchHelper, and the dispatch in clearSearch.
The mainSlice was inconsistent: most actions used `set` + PascalCase (setSearchResults, setShowZoomNotice) but 26 used `set` + camelCase prefix (setisDrawingEnabled, setappConfig, setmappedScenes, etc.). Renamed all 26 to the PascalCase convention. State field names unchanged. Pure mechanical rename across 52 files; no behavior change. Also: in CollectionDropdown, change ` git diff src/redux/slices/mainSlice.js | head -60 && echo --- && git diff src/components/CollectionDropdown/CollectionDropdown.jsxselectedCollectionData` to ` git diff src/redux/slices/mainSlice.js | head -60 && echo --- && git diff src/components/CollectionDropdown/CollectionDropdown.jsxselectedCollectionData?.id` so the effect body's read matches the dependency array, avoiding a subtle exhaustive-deps inconsistency.
clearSearch dispatched 16 sequential actions to reset every search-derived state field. Each dispatch ran every subscriber selector, causing React to schedule and batch ~16 reconciliation passes per call. Replace with a single `resetSearchState` compound reducer in mainSlice that performs the same field resets atomically. The reducer is documented with the persistent-UI fields it deliberately leaves untouched (selectedCollection, viewMode, theme, etc.). clearSearch now: clear map layers \u2192 dispatch resetSearchState() \u2192 navigate. Same fields cleared, same URL transitions, same observable behavior; one dispatch instead of 16.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Co-authored-by: Copilot <copilot@github.com>
Preserve map overlays across remount by detaching data layers without clearing them, while still clearing ephemeral layers. Use one-time active-router snapshots for mount-only URL seeds in LeafMap and ViewSelector to avoid reactive useSearch subscriptions. Migrate RightContent test setup to the renderFilmDrop harness with a fresh store and keep module-scope router mocks.
- Remove redundant 'Reads colors from CSS' comments in mapStyles - Fix VisualizationDropdown import consolidation and accessibility - Add missing useEffect dependency (setViz) - Associate form label with checkbox control via htmlFor/id - Delete mapHelper barrel; migrate imports to direct modules - App.jsx and mapHelper.test.js now import from mapLayers/mapStyles All 768 tests passing. Quality gates clean (lint, format, typecheck).
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.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
2 participants
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
This branch reframes FilmDrop UI from a single standalone application into a first-class, npm-distributable React component library that still supports a local SPA dev/demo entry. Practically, that means a published
distbundle, explicitpackage.jsonexports(JS + types + CSS), peer dependencies for the host app’s React/MUI/Redux/Router/Leaflet stack, a library entry separate from the SPA entry, and consumer-facing docs, starter app, and verification scripts so teams can embed FilmDrop inside their own apps, not only run the repo as its own site.Alongside that packaging shift, the branch includes substantial hardening work (services, map module split, Redux/router correctness, tests, CI) that supports reliable embedding in foreign hosts and stricter library boundaries.
Changes (high level)
filmdrop-uias an installable package withmain/module/types,exportsmap, andfileslimited todist+ key docs, not “clone and run” as the only consumption story.index.jsx+ Leaflet CSS at the app boundary) vs library bundle (lib-entry/ Vite lib build) so Leaflet/CSS ownership and bundling match how consumers integrate.peerDependencies(React, MUI, Redux Toolkit, TanStack Router, Leaflet/React-Leaflet, etc.) so the library fits normal app supply chains.FilmDropRootand restructureAppso the UI is composed as a mountable subtree suitable for embedding, not only a monolithic app shell..d.tsso TypeScript apps can import the package cleanly.examples/starterand scripts to validate types, library bundle, and consumer smoke flows; evidence the component works outside this repo’s original app layout.renderFilmDrop, header factories, etc.).config,urlState, andonUrlStateChangesupport toFilmDropRoot.