2035-validate-filters-form-load-sr#2062
Conversation
…fetched and loaded
…rs data to be downloaded
… selection via ReactDayPicker
RoanBox
left a comment
RoanBox
left a comment
There was a problem hiding this comment.
Leaving REQUEST_CHANGES based on needed fixes to the blank-map data-loading flow, address/council interaction behavior, and the Export enablement logic in this PR.
| this.processSearchParams(); | ||
| await this.createRequestsTable(); | ||
| await this.setData(); | ||
| // Blank Map Implementation: createRequestTable and setData now called via Redux |
There was a problem hiding this comment.
This changes the initial load flow, but there are still app paths assuming the DuckDB table exists on startup. Running this locally, the app throws from LastUpdated.jsx with Table with name requests_2025 does not exist! before any Display Data action. The delayed-load flow needs to be made consistent across startup consumers before this is ready to merge.
| geoFilterType={geoFilterType} | ||
| councils={councils} | ||
| onGeocoderResult={onGeocoderResult} | ||
| onGeocoderResult={handleGeocoderResult} |
There was a problem hiding this comment.
FilterMenu now intercepts the geocoder result here, but it no longer forwards that result to the existing parent callback from Map.jsx. That appears to bypass the current address-search behavior (reset/zoom/council selection) rather than just delaying data fetch until submit.
| dispatchUpdateSelectedCouncils(newSelected); | ||
| dispatchUpdateUnselectedCouncils(councils); | ||
| dispatchUpdateNcId(selectedCouncilId); | ||
| // dispatchUpdateNcId(selectedCouncilId); // Triggers zoom |
There was a problem hiding this comment.
These calls are commented out, so selecting a council no longer updates ncId or closes the boundaries picker. Since map zoom behavior changes are explicitly out of scope for this issue, this looks like an unrelated behavior regression.
| <div className={`${classes.selectorWrapper} ${classes.export}`}> | ||
| <ExportButton /> | ||
| <div className={classes.selectorWrapper}> | ||
| <ExportButton className={classes.export} disabled={formValid === false}/> |
There was a problem hiding this comment.
This enables Export based on local form validity, not on whether data has actually been fetched and loaded successfully. The issue requirement says Export should remain disabled until data is successfully fetched and loaded.
|
This branch conflicts with Merge conflict context and recommended resolutionI walked the merge locally to see what kind of conflict this actually is. The good news is that the merge only produces a direct content conflict in one file, The important part is that this is not a simple “pick ours vs. pick theirs” conflict. 1. What changed on On the Two recent PRs matter here. PR 2. What your PR is trying to do On this PR’s side, Specifically, it comments out the startup calls to That is a legitimate direction for this ticket, but it is being layered onto a stale copy of 3. What changed and why it conflicted There is also longer-running feature context here. The Blank Map/request-table direction traces back to PR So conceptually, this PR is moving in a direction that has precedent in the project. The problem is that Recommended resolution Because of that, I would recommend resolving the conflict by starting from the current In practice, that means preserving the current That last point matters because I tested this locally and hit a real startup failure after following this PR’s flow: After the conflict is resolved, I’d re-test initial page load, |
2 participants
Fixes Issue #2035 (DEV - validate filters form and load SR data on form submit)
Status
Implemented Changes
Action Items
mainbranch