refactor(ui): extract shared TagsSection for devices and containers#6644
Merged
Conversation
|
Claude finished @luizhf42's task in 5m 44s —— View job Code Review CompleteReviewed 5 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is. A few things I specifically verified during the review:
Behavioral changes (containers gaining autocomplete, devices gaining visible remove-error messages, ARIA combobox navigation) are intentional merges of the two prior implementations, not regressions. If you push additional changes and want a new review, tag |
Unify the duplicated `TagsSection` implementations — the extracted component in `pages/devices/TagsSection.tsx` and the inline copy in `ContainerDetails.tsx` — into a single shared component at `components/common/TagsSection.tsx`. The shared component accepts `addTag`/`removeTag` mutation functions as props, keeping it entity-agnostic. It merges the device version's search/autocomplete dropdown with the container version's `isSdkError`-based error handling. Containers gain the autocomplete feature and devices gain visible remove-error messages. Also adds WAI-ARIA combobox keyboard navigation (ArrowDown/ArrowUp to cycle options, Enter to select, `aria-activedescendant` for screen readers), a double-submit guard, and `scrollIntoView` on the active option. Replaces local `LABEL` constants with the shared `LABEL_BASE` from `@/utils/styles`.
30f74c1 to
0886852
Compare
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
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.
What
Unified the duplicated
TagsSectionimplementations — the extracted component inpages/devices/TagsSection.tsxand the inline copy inContainerDetails.tsx— into a single shared component atcomponents/common/TagsSection.tsxthat acceptsaddTag/removeTagmutation functions as props.Why
Both detail pages had near-identical tag management UI with the same validation rules, styling, and mutation call signature (
{ path: { uid, name } }). The device version had a search/autocomplete dropdown the container version lacked; the container version hadisSdkError-based error messages the device version lacked. Maintaining both meant divergent features and duplicated bug surface.Changes
components/common/TagsSection.tsx: new shared component merging the best of both versions — the device version's autocomplete dropdown (powered by the namespace-wideuseTags()hook) and the container version's status-specific error handling (isSdkErrorfor 403/400). Added WAI-ARIA combobox keyboard navigation (ArrowDown/ArrowUp to cycle options, Enter to select,aria-activedescendantfor screen readers), a double-submit guard inhandleAdd,scrollIntoViewon the active option, andcloseDropdownhelper that resets bothopenandactiveIndexstate. Dropdown items userole="listbox"/role="option"withonMouseDown+preventDefaultto avoid input blur race.pages/DeviceDetails.tsx: imports sharedTagsSection, liftsuseAddDeviceTag/useRemoveDeviceTagto page level, passesmutateAsyncas props.pages/ContainerDetails.tsx: deleted the ~120-line inlineTagsSection, imports shared version with container mutations. Removed unusedTagIcon,PlusIcon,useHasPermissionimports. Replaced localLABELconstant with sharedLABEL_BASEfrom@/utils/styles.pages/devices/TagsSection.tsx: deleted — replaced by the shared component.components/common/__tests__/TagsSection.test.tsx: 22 tests covering rendering, permission gating, add/remove with validation, API error differentiation (403/400), autocomplete suggestions, keyboard navigation (ArrowDown + Enter, wrap-around), and accessibility (role="alert", descriptivearia-labels).Testing
npm run test— 22/22 TagsSection tests pass