refactor(navItem): "correctly" disabling links #24
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -26,12 +26,13 @@ const NavItem = ({ | |||||
| icon={icon} | ||||||
| key={`router-leaf-${target ?? title}`} | ||||||
| disabled={disabled} | ||||||
| > | ||||||
| <a | ||||||
| href={trimExtension(target)} | ||||||
| aria-current={isActive ? 'page' : undefined} | ||||||
| data-icon-trailing={iconTrailing} | ||||||
| tabindex={disabled ? '-1' : undefined} | ||||||
| aria-disabled={disabled ? 'true' : undefined} | ||||||
| > | ||||||
|
Comment on lines
30
to
36
There was a problem hiding this comment. When a link is disabled, it should not be navigable via click. Consider adding an onClick handler that prevents navigation when disabled, or use CSS pointer-events to prevent interaction. Without this, users can still navigate to the disabled link by clicking on it, defeating the purpose of disabling it.
Contributor
There was a problem hiding this comment. Copilot is right. Just adding |
||||||
| {title} | ||||||
| </a> | ||||||
|
|
@@ -61,12 +62,13 @@ const NavItem = ({ | |||||
| icon={icon} | ||||||
| key={`router-leaf-${path ?? title}`} | ||||||
| disabled={disabled} | ||||||
| > | ||||||
| <a | ||||||
| href={trimExtension(path)} | ||||||
| aria-current={getAriaCurrent(trimExtension(path))} | ||||||
| data-icon-trailing={iconTrailing} | ||||||
| tabindex={disabled ? '-1' : undefined} | ||||||
|
||||||
| tabindex={disabled ? '-1' : undefined} | |
| tabIndex={disabled ? '-1' : undefined} |
In React/JSX, the attribute should be tabIndex (camelCase) instead of tabindex (lowercase). React will issue a warning about this in the console, and the lowercase version may not work correctly in all scenarios.
There was a problem hiding this comment.
When a link is disabled, it should not be navigable via click. Consider adding an onClick handler that prevents navigation when disabled, or use CSS pointer-events to prevent interaction. Without this, users can still navigate to the disabled link by clicking on it, defeating the purpose of disabling it.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In React/JSX, the attribute should be
tabIndex(camelCase) instead oftabindex(lowercase). React will issue a warning about this in the console, and the lowercase version may not work correctly in all scenarios.