Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JNG-5983 tag navigation #477

Merged
merged 1 commit into from
Oct 29, 2024
Merged

JNG-5983 tag navigation #477

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions judo-ui-react/src/main/resources/actor/src/components/widgets/Tags.tsx.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,13 @@ export function Tags<P, T>(props: TagsProps<P, T>) {
renderTags={(tagValue, getTagProps) =>
tagValue.map((option, index) => {
const { key, ...rest } = getTagProps({ index });
return (
<Chip
key={key}
label={option[autoCompleteAttribute]}
{...rest}
/>
);
return <Chip
key={key}
label={option[autoCompleteAttribute]}
{...rest}
sx={ { cursor: (typeof onItemClick === 'function' && !editMode) ? 'pointer' : undefined } }
onClick={onChipClicked}
/>;
Comment on lines +212 to +218
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using data attributes for more reliable click handling.

The current implementation relies on textContent to find the clicked tag, which could be fragile if the label text is not unique. Consider using data attributes for more reliable click handling.

Here's a suggested improvement:

 return <Chip
   key={key}
   label={option[autoCompleteAttribute]}
   {...rest}
   sx={ { cursor: (typeof onItemClick === 'function' && !editMode) ? 'pointer' : undefined } }
-  onClick={onChipClicked}
+  data-tag-id={option[identifierAttribute]}
+  onClick={(event) => {
+    if (onItemClick && !editMode) {
+      const tagId = (event.currentTarget as HTMLElement).dataset.tagId;
+      const data = values.find((c: any) => c[identifierAttribute] === tagId);
+      if (data) {
+        onItemClick(data);
+      }
+    }
+  }}
 />;

This approach:

  • Uses data attributes to store the tag identifier
  • Moves the click logic inline for better type safety
  • Removes the need for textContent matching

Committable suggestion was skipped due to low confidence.

🛠️ Refactor suggestion

Enhance accessibility for clickable tags.

When tags are clickable (onItemClick present and not in edit mode), they should be properly marked as interactive elements for accessibility.

Add these accessibility attributes:

 return <Chip
   key={key}
   label={option[autoCompleteAttribute]}
   {...rest}
   sx={ { cursor: (typeof onItemClick === 'function' && !editMode) ? 'pointer' : undefined } }
   onClick={onChipClicked}
+  role={onItemClick && !editMode ? "button" : undefined}
+  tabIndex={onItemClick && !editMode ? 0 : undefined}
+  onKeyDown={(e) => {
+    if (onItemClick && !editMode && (e.key === 'Enter' || e.key === ' ')) {
+      e.preventDefault();
+      onChipClicked(e as any);
+    }
+  }}
 />;

This enhancement:

  • Adds proper role for screen readers
  • Enables keyboard navigation
  • Handles keyboard events (Enter/Space) for activation
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return <Chip
key={key}
label={option[autoCompleteAttribute]}
{...rest}
sx={ { cursor: (typeof onItemClick === 'function' && !editMode) ? 'pointer' : undefined } }
onClick={onChipClicked}
/>;
return <Chip
key={key}
label={option[autoCompleteAttribute]}
{...rest}
sx={ { cursor: (typeof onItemClick === 'function' && !editMode) ? 'pointer' : undefined } }
onClick={onChipClicked}
role={onItemClick && !editMode ? "button" : undefined}
tabIndex={onItemClick && !editMode ? 0 : undefined}
onKeyDown={(e) => {
if (onItemClick && !editMode && (e.key === 'Enter' || e.key === ' ')) {
e.preventDefault();
onChipClicked(e as any);
}
}}
/>;

})
}
renderInput={(params) => (
Expand Down Expand Up @@ -261,10 +261,6 @@ export function Tags<P, T>(props: TagsProps<P, T>) {
} }
/>
)}
ChipProps={ {
clickable: typeof onItemClick === 'function' && !editMode,
onClick: onChipClicked,
} }
/>
);
}
Loading