feat(workbench): various UX improvements#8165
Conversation
Triggered by e922695 on branch refs/heads/issue-8153
Update terminology and user-facing copy from 'Upload Plan' to 'Mapping' (or 'Mapping Plan') across the WorkBench.
Triggered by ec3b3c0 on branch refs/heads/issue-8153
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Triggered by e3fb771 on branch refs/heads/issue-8153
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
specifyweb/frontend/js_src/lib/components/WbActions/useResults.ts (1)
40-77: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRe-show previously filtered rows/columns before applying the new result set.
This effect now reruns on
cellCountschanges, but it only hides. If a later validation/update makes a row or column contain results, it stays hidden from the earlier run until the panel is closed, so the open results view can only shrink.Possible fix
React.useEffect(() => { if (hot === undefined || !showResults) return; + const hiddenRows = getHotPlugin(hot, 'hiddenRows'); + const hiddenColumns = getHotPlugin(hot, 'hiddenColumns'); const rowsToInclude = new Set<number>(); const colsToInclude = new Set<number>(); Object.entries(workbench.cells.cellMeta).forEach(([physicalRow, rowMeta]) => rowMeta.forEach((metaArray, physicalCol) => { @@ hot.batch(() => { identifyDefaultValues(hot, workbench.mappings); - getHotPlugin(hot, 'hiddenRows').hideRows(rowsToHide); - getHotPlugin(hot, 'hiddenColumns').hideColumns(colsToHide); + hiddenRows.showRows( + hiddenRows + .getHiddenRows() + .filter((row) => !initialHiddenRows.includes(row)) + ); + hiddenColumns.showColumns( + hiddenColumns + .getHiddenColumns() + .filter((column) => !initialHiddenCols.includes(column)) + ); + hiddenRows.hideRows(rowsToHide); + hiddenColumns.hideColumns(colsToHide); workbench.utils.toggleCellTypes('newCells', 'remove'); }); }, [showResults, cellCounts]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/frontend/js_src/lib/components/WbActions/useResults.ts` around lines 40 - 77, The `useResults` effect only hides rows/columns and never restores ones that were hidden by a previous run, so the results view can only shrink as `cellCounts` changes. Update the `React.useEffect` in `useResults` to re-show the previously hidden rows/columns before computing and applying the new hidden sets, using the existing `hiddenRows` and `hiddenColumns` plugins around the `identifyDefaultValues`, `hideRows`, and `hideColumns` calls. Keep the existing `showResults`, `cellCounts`, `hot`, `initialHiddenRows`, and `initialHiddenCols` logic intact while ensuring newly result-containing rows/columns become visible again.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@specifyweb/frontend/js_src/lib/components/WbActions/useResults.ts`:
- Around line 40-77: The `useResults` effect only hides rows/columns and never
restores ones that were hidden by a previous run, so the results view can only
shrink as `cellCounts` changes. Update the `React.useEffect` in `useResults` to
re-show the previously hidden rows/columns before computing and applying the new
hidden sets, using the existing `hiddenRows` and `hiddenColumns` plugins around
the `identifyDefaultValues`, `hideRows`, and `hideColumns` calls. Keep the
existing `showResults`, `cellCounts`, `hot`, `initialHiddenRows`, and
`initialHiddenCols` logic intact while ensuring newly result-containing
rows/columns become visible again.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 338596a0-264d-4203-936f-6597d3bdc594
📒 Files selected for processing (10)
specifyweb/frontend/js_src/lib/components/Header/ExpressSearchTask.tsxspecifyweb/frontend/js_src/lib/components/WbActions/WbNoUploadPlan.tsxspecifyweb/frontend/js_src/lib/components/WbActions/WbValidate.tsxspecifyweb/frontend/js_src/lib/components/WbActions/useResults.tsspecifyweb/frontend/js_src/lib/components/WbPlanView/Mapper.tsxspecifyweb/frontend/js_src/lib/components/WbToolkit/DevShowPlan.tsxspecifyweb/frontend/js_src/lib/components/WorkBench/WbView.tsxspecifyweb/frontend/js_src/lib/localization/batchEdit.tsspecifyweb/frontend/js_src/lib/localization/wbPlan.tsspecifyweb/frontend/js_src/lib/localization/workbench.ts
✅ Files skipped from review due to trivial changes (2)
- specifyweb/frontend/js_src/lib/localization/batchEdit.ts
- specifyweb/frontend/js_src/lib/localization/workbench.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- specifyweb/frontend/js_src/lib/components/WbPlanView/Mapper.tsx
- specifyweb/frontend/js_src/lib/components/WbActions/WbNoUploadPlan.tsx
- specifyweb/frontend/js_src/lib/localization/wbPlan.ts
- specifyweb/frontend/js_src/lib/components/WbToolkit/DevShowPlan.tsx
|
Re: @emenslin
This should be fixed.
This was fixed by using CodeMirror as we do in App Resources here.
I can't recreate this, but it still falls outside the scope of this PR (ostensibly, at least). Can you open a bug if you can recreate this consistently? I think it has to do with that specific agent being created in the same data set in two different roles (e.g. determiner, collector) twice.
This is intentional, as far as I can tell. That number represented the number of rows still to validate.
Good idea, implemented:
No, I'd like to keep it distinct. It's not a bad suggestion, but this is an intentional design decision. If you clear the value from the editor, you can actually drag a file into it: Screen.Recording.2026-06-29.at.7.47.23.PM.mov
Can you open a new issue for this? This is an existing bug. |
emenslin
left a comment
emenslin
left a comment
There was a problem hiding this comment.
1. Live Validation & Upload Record Counts (Fixes #8153)
- Verify that "Live Validation" is enabled by default.
- Verify that the validation and upload confirmation/results dialogs show a table outlining the records created, updated, deleted, or matched/changed, depending on your dataset operations.
2. Mapping Import/Export & Clear Confirmation
- Verify that you can Export the current plan as a
.jsonfile. - Test the Import feature by selecting a
.jsonfile from your local machine. - Verify that uploading a corrupt or invalid JSON file presents an invalid JSON error dialog.
- Click Clear Mappings and ensure a confirmation dialog appears asking to verify erasing the mappings before proceeding.
- Switch datasets to one without a mapping. Verify that the dialog now includes a button to Choose Existing Mapping.
3. Base Table Selection Dialog
- Verify the dialog width/height scales properly and that it displays descriptions clarifying what a "base table" is.
4. SvgIcon Alignment in Firefox (Fixes #8155)
- Verify that column/table status icons align correctly and do not shift out of place.
5. Express Search Dialog Shifting (Fixes #8014)
- Verify the dialog does not shift layout when opened/closed.
After a live validation if you open the results panel and then close it, all the results go away along with all the highlighted cells, although the counts still show.
06-30_11.38.mp4
I did run into an issue where sometimes if you try to disambiguate it doesn't work. This one wasn't super consistent so I'm not sure why it was working sometimes and not others. When I could get it to work, I sometimes ran into my issue from my last review where some were disambiguated but others were saying they needed to be disambiguated but I couldn't disambiguate it. I know before you said it was out of the scope of this PR but since the behavior is different now I figured I would review it here first.
06-30_11.31.mp4
Here's a link to the smaller data set I was using for testing: https://ojsmnh20260526-issue-8153.test.specifysystems.org/specify/workbench/842/
Triggered by 9171678 on branch refs/heads/issue-8153
|
@emenslin Fixed the results disappearing! I ask that you open a new issue for any of the disambiguation issues remaining, unless you cannot recreate when compared against |
4 participants
Fixes #8153
Fixes #8155
Fixes #8014
Fixes #4484
This PR introduces a number of different UI improvements, localization updates, and bug fixes centered around the WorkBench data mapping and upload workflows:
Terminology Updates ("Upload Plan" ➔ "Mapping"):
chooseExistingPlanbecomes "Choose Existing Mapping",uploadPlanis now "Import/Export Mapping").Record Counts & Live Validation:
TableRecordCountscomponent.dataCheck) by default (defaultValue: truein user preferences).Import/Export Mapping Enhancements:
.jsonfile in addition to exporting.Data Mapping Dialog & Usability Improvements:
WorkBench
WorkBench Attachments
UI Fixes:
dy=".35em"on<text>elements inSvgIcon.tsxto fix icon rendering bugs in Firefox.dialogClassNames.extraWideContainerto prevent container shifting and clipping.Checklist
self-explanatory (or properly documented)
specify7/specifyweb/specify/management/commands/run_key_migration_functions.py
Line 50 in ea04665
Testing instructions
1. Live Validation & Upload Record Counts (Fixes #8153)
2. Mapping Import/Export & Clear Confirmation
.jsonfile..jsonfile from your local machine. Verify that uploading a corrupt or invalid JSON file presents an invalid JSON error dialog.3. Base Table Selection Dialog
4. SvgIcon Alignment in Firefox (Fixes #8155)
5. Express Search Dialog Shifting (Fixes #8014)
Summary by CodeRabbit
Summary
New Features
Improvements
Bug Fixes