Repository navigation
Upgrade Parent Tag Configuration handling for Accessibility - #4254
Conversation
- Rename the tag context menu action to "Edit" and let it change the name and the parent tag in one modal, so a tag can be moved without dragging - Offer every tag as a destination, labelled with its full path, rather than only tags that already have children. Indentation is never announced, so the path is what tells a screen reader where a tag sits - Leave out the edited tag and its subtree, which cannot be valid targets - Filter on the whole path, so typing a parent name finds the tags below it - Translate the new text into all nine languages and drop the two translation keys the rename modal no longer uses Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Filtered tree data hides valid move destinations, and the new modal regresses Enter-to-save behavior in the name field.
2 open issues
What changed in this PR
Verdict: Needs changes. This PR addresses #942 by adding a keyboard-accessible alternative to drag-and-drop tag moves within Tag Configuration.
Changes:
- Replaces Rename with an Edit modal that saves the tag name and parent together.
- Adds searchable, full-path destinations while excluding the edited tag and its descendants.
- Adds utility tests and updates translations across nine languages.
The behavior belongs in the tags module, and no shared API signatures change. Utility tests cover destination generation, but interactive regression coverage remains incomplete. No documentation or changelog changes are included; runtime keyboard behavior was not tested during this review.
| File | Description |
|---|---|
| translations/studio.sv.yaml | Adds Swedish edit/move text; removes rename keys. |
| translations/studio.pl.yaml | Adds Polish edit/move text; removes rename keys. |
| translations/studio.no.yaml | Adds Norwegian edit/move text; removes rename keys. |
| translations/studio.nl.yaml | Adds Dutch edit/move text; removes rename keys. |
| translations/studio.it.yaml | Adds Italian edit/move text; removes rename keys. |
| translations/studio.fr.yaml | Adds French edit/move text; removes rename keys. |
| translations/studio.es.yaml | Adds Spanish edit/move text; removes rename keys. |
| translations/studio.en.yaml | Adds English edit/move text; removes rename keys. |
| translations/studio.de.yaml | Adds German edit/move text; removes rename keys. |
| assets/js/src/core/modules/tags/utils/move-target-options.ts | Builds and filters full-path move destinations. |
| assets/js/src/core/modules/tags/utils/move-target-options.test.ts | Tests paths, subtree exclusion, and matching. |
| assets/js/src/core/modules/tags/tag-configuration-container.tsx | Connects the Edit action and combined save flow. |
| assets/js/src/core/modules/tags/components/edit-tag-modal/edit-tag-modal.tsx | Adds the name-and-parent editing modal. |
Review effort: Balanced
| name="name" | ||
| rules={ [{ required: true, message: t('tag-configuration.name-required') }] } | ||
| > | ||
| <Input /> |
There was a problem hiding this comment.
Fixed in 7b5cf19.
You are right that implicit submission never fires here: the form holds two fields that block it (the name input and the search input the destination select renders), and the Save button sits in the modal footer, outside the form, so it is no submit button of the form either. I confirmed both in a test before fixing.
The name field now handles Enter itself, <Input onPressEnter={ handleSave } />, matching how the other modal forms in this codebase restore Enter (for example use-add-document.tsx). I kept it explicit rather than associating the footer button with the form via form/htmlType="submit", because that would have made the button the form's default button and fired handleSave twice, once from the click and once from the submit.
Regression tests in edit-tag-modal.test.tsx:
saves on Enter in the name field, as the rename dialog did- fails on the previous code and passes nowleaves the form to the explicit handler rather than to implicit submission- pins the two inputs and the absence of a submit button, so the reason Enter needs explicit handling is recordeddoes not save on Enter while the name is empty- validation still blocks the save
The test renders the real AntD form, input and select that the Studio components wrap, since the Studio ones reach antd-style's untranspiled ESM and pull in the whole app import tree under jest.
| } } | ||
| rootTag={ rootTagFolder } | ||
| tag={ tagToEdit } | ||
| tags={ tags } |
There was a problem hiding this comment.
Fixed in 7b5cf19.
Confirmed: the modal was given tags from useTagConfig(), and that collection is fetched with the toolbar filter, so searching the tree for Germany really did remove Quality from "Move under", and the modal's own search could not bring back what the collection never held.
useTagConfig now runs a second useTagGetCollectionQuery with filter: '' and exposes it as moveTargetTags, with moveTargetTagsFetching and moveTargetTagsFailed alongside. The tree keeps following the toolbar search; only the destinations ignore it. While the toolbar is empty both queries have identical arguments, so RTK Query serves them from one cache entry and this costs no extra request.
Loading and errors are handled in the modal:
- while the collection is in flight the select is disabled and shows its loading state
- if it fails, the select is disabled and the failure is the field's accessible description ("The list of tags could not be loaded, so only the name can be changed"), translated into all nine languages. The tag stays renameable
A side effect worth noting: the initial parentId is now always present among the options, so the select no longer risks displaying a bare id when the current parent was filtered out.
Regression tests in use-tag-config.test.tsx:
keeps the move destinations out of the toolbar search- sets the filter to Germany, then asserts the tree collection narrows to Countries whilemoveTargetTagsstill holds Countries and Quality, and that the collection is queried with an empty filteroffers a destination the tree search hides- with that same filter active, builds the options as the modal does and asserts "All Tags / Quality" is among them
I put these on the hook rather than on the container because that is where the filter was leaking into the destinations; the container test would have had to mock useTagConfig and so would have mocked away the bug.
- Restore Enter-to-save in the name field. The Save button sits in the modal footer, outside the form, and the destination select adds a second input to it, so the browser never submits the form implicitly. The name field now handles Enter itself, as the other modal forms in the codebase do - Load the move destinations with an empty filter, independently of the tree toolbar search. Searching the tree for one tag used to drop every unrelated tag from the collection the modal builds its destinations from, and the modal's own search could not bring back what was never there - Show the destination select as loading while that collection is in flight, and name the failure as the field's accessible description when it cannot be loaded, leaving the tag renameable - New message translated into all nine languages - Regression tests: Enter in the name field saves, the form carries no submit button of its own, validation still blocks an empty name, and the destinations ignore an active tree filter Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Moving a tag was only possible by dragging it onto another tag, which leaves out anyone who cannot use drag and drop. The tag context menu action is now "Edit" and changes the name and the parent in one modal.
The flow works by keyboard throughout:
Shift+F10opens the tag's context menu,Tabmoves into it, arrows reach "Edit", and the modal is operated with Tab, typing and Enter. The select reportsrole="combobox"witharia-expanded,aria-autocomplete="list"andaria-activedescendant.Two accessibility gaps remain, both outside this change:
TabafterShift+F10; arrow keys alone do not move into it. That lives in the sharedTreeElementResolves #942
🤖 Generated with Claude Code