-
-
Notifications
You must be signed in to change notification settings - Fork 609
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.
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
Focus visible style refinements #1577
base: main
Are you sure you want to change the base?
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/webapp/app/components/primitives/Table.tsxOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces a series of updates across multiple UI components in the web application, focusing on CSS class name modifications, enhanced focus styles, and improved interactivity. Key components affected include Input, SimpleSelect, Table, and TextLink, with corresponding adjustments made to the Tailwind configuration to support these changes. The updates aim to enhance accessibility and create a more consistent user interface without altering the core functionality of the components. Changes
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@trigger.dev/build
@trigger.dev/react-hooks
trigger.dev
@trigger.dev/rsc
@trigger.dev/sdk
@trigger.dev/core
commit: |
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/webapp/app/components/primitives/Table.tsx (3)
113-117
: Ensure that disabled rows remain truly non-interactive.You set tabIndex to -1 for disabled rows and 0 otherwise, but you’re also setting onClick and onKeyDown. If “disabled” truly means non-interactive, consider removing these event handlers or preventing them from doing anything when disabled is true.
onClick={disabled ? undefined : handleInteraction} onKeyDown={disabled ? undefined : handleKeyDown}
163-163
: Revisit negative tabIndex on header cells.Similar to line 47, confirm that negative tabIndex is the intended way to skip focus on TableHeaderCell. This can be acceptable, but it’s worth verifying against your accessibility goals.
192-192
: Row hover & focus styling approach.The new focus and hover style classes align with the “Focus visible” changes described in the PR. Nice consistency. Beware potential overlap between :hover, :focus, and :focus-visible states for color or background clashes on older browsers.
apps/webapp/tailwind.config.js (1)
Line range hint
308-316
: Well-implemented focus utility using :focus-visibleThe
.focus-custom
utility correctly uses:focus-visible
instead of:focus
, which is a best practice as it:
- Only shows focus indicators for keyboard navigation
- Hides focus indicators when clicking/tapping
- Provides consistent focus styles across components
Consider documenting this utility in your component library or style guide to encourage consistent usage across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/components/primitives/Input.tsx
(2 hunks)apps/webapp/app/components/primitives/SimpleSelect.tsx
(1 hunks)apps/webapp/app/components/primitives/Table.tsx
(6 hunks)apps/webapp/app/components/primitives/TextLink.tsx
(2 hunks)apps/webapp/tailwind.config.js
(1 hunks)
🔇 Additional comments (14)
apps/webapp/app/components/primitives/Table.tsx (7)
2-2
: Nice addition for navigation!
Importing useNavigate from Remix allows dynamic navigation within the table row interactions. This is consistent with the new pattern of passing a “to” prop to navigate.
47-47
: Clarify why the table header uses tabIndex={-1}.
This makes the header non-tabbable, which could be intentional for accessibility. However, confirm that this is the desired behavior since it prevents keyboard users from focusing on the header.
75-76
: Good approach for adding navigation and click handlers.
Adding “to” and “onClick” props expands the row’s interactivity. Just be sure downstream usage enforces consistent typing for event handling across row-based navigations.
102-108
: Keyboard accessibility looks great.
Handling the “Enter” key to trigger the same logic as onClick is a clean approach and ensures compliance with accessible interactions. Great job here.
236-237
: Conditional cursor-pointer is a good pattern.
Assigning “cursor-pointer” only when hasAction is true prevents confusion for non-clickable cells. Good approach for usability.
245-245
: Directly rendering children is simpler.
By eliminating conditional logic wrapping Link or button, you now rely on the row for navigation or the consumer to handle wrapping. This is cleaner, but confirm that all needed ARIA attributes are present if these children represent interactive elements.
80-101
: Potential child type checking caveat.
You’re using child.type === TableCell to detect cells with a “to” prop. This could fail if TableCell is wrapped by HOCs or if multiple React instances are in play.
If you want to confirm that no HOC or different React instance is wrapping TableCell, run the following:
apps/webapp/app/components/primitives/TextLink.tsx (2)
7-7
: LGTM: Focus-visible styles added consistently
The addition of focus-visible:focus-custom
to both primary and secondary variants aligns with the PR objectives for more refined focus states.
Also applies to: 9-9
37-37
: LGTM: Icon sizing updated to modern notation
The change from h-4 w-4
to size-4
follows modern Tailwind practices for consistent sizing across components.
Also applies to: 44-44
apps/webapp/app/components/primitives/Input.tsx (2)
7-7
: LGTM: Focus ring refinements align with objectives
The changes to focus ring styles:
- Reduced thickness from ring-2 to ring-1
- Updated color to ring-charcoal-650 for subtler focus states
These changes align perfectly with the PR objectives for more refined focus states.
20-20
: LGTM: Consistent icon sizing across variants
Updated all icon size classes to use the modern size-X
notation, maintaining consistency across all variants and aligning with the changes in TextLink component.
Also applies to: 27-27, 34-34, 40-40
apps/webapp/app/components/primitives/SimpleSelect.tsx (1)
32-32
: LGTM: Focus-visible state added as per objectives
Replaced explicit ring styles with focus-visible:focus-custom
, which:
- Adds the missing focus-visible state as specified in PR objectives
- Maintains consistency with other components using the same focus style
apps/webapp/tailwind.config.js (2)
215-218
: LGTM: Focus style changes align with PR objectives
The changes successfully implement more subtle focus styles by:
- Reducing outline thickness from 2px to 1px
- Using the textLink color for consistent branding
- Adjusting border radius for better visual harmony
Let's verify the impact on components mentioned in the PR:
✅ Verification successful
Focus style changes successfully implemented across components
The changes to focus styles in tailwind.config.js have been properly implemented and are being used consistently:
- SimpleSelect and TextLink components now use
focus-visible:focus-custom
as required - Input component has updated focus styles with 1px ring thickness
- The changes are applied across multiple primitive components ensuring consistent focus states
- The textLink color is properly used through the focus-custom utility class
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using focus-custom styles
rg -l "focus-custom" --type tsx
Length of output: 59
Script:
#!/bin/bash
# Search for components using focus-custom styles with broader file types
rg -l "focus-custom" --type ts --type jsx --type js --type tsx
# Also search for focus-visible related styles as mentioned in PR objectives
rg -l "focus-visible" --type ts --type jsx --type js --type tsx
# Search in CSS/SCSS files as well
rg -l "focus-custom|focus-visible" --type css --type scss
Length of output: 271
Script:
#!/bin/bash
# Search for components using focus-custom styles without specifying file types
rg -l "focus-custom"
# Search for focus-visible related styles
rg -l "focus-visible"
# Use fd to find relevant files first
fd -e tsx -e jsx -e ts -e js -e css -e scss
Length of output: 67219
Script:
#!/bin/bash
# Search for components using focus-custom and focus-visible styles in the codebase
rg -l "focus-custom|focus-visible" apps/webapp/app/components/
# Check the content of SimpleSelect and TextLink components mentioned in PR objectives
rg -A 5 "focus-custom|focus-visible" apps/webapp/app/components/primitives/SimpleSelect.tsx apps/webapp/app/components/primitives/TextLink.tsx
# Check the Task input component for focus state changes
rg -A 5 "focus-custom|focus-visible" apps/webapp/app/components/primitives/Input.tsx
Length of output: 3871
Line range hint 8-31
: Verify contrast ratios for focus indicators
The new lavender[400] color (#826dff) is used for focus indicators. While the color choice looks good, we should verify it meets WCAG accessibility guidelines for focus indicators.
Let's check the contrast ratios:
Also applies to: 89-108
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/webapp/app/components/primitives/Table.tsx (4)
83-101
: Refactor row-click logic for clarity.The handleInteraction function is robust, but it nests multiple conditions. Consider extracting the button/link detection and path resolution logic into smaller helper functions to clarify the flow and reduce complexity.
102-109
: Consider supporting Space key activation.Only triggering on Enter is standard, but some users also expect Space to activate the focused row. Consider extending the handleKeyDown to detect Space for improved accessibility.
117-117
: Verify focus-visible style with custom backgrounds.The focus-visible:bg-background-bright class is effective for emphasizing the row. Just confirm the color doesn't clash with selected or hover states.
234-236
: Cursor pointer usage.Conditionally applying cursor-pointer only when hasAction is true preserves the expected visual clue. Ensure you also handle pointer events if the row is disabled or if no actual navigation occurs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/primitives/Table.tsx
(6 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.alerts/route.tsx
(1 hunks)
🔇 Additional comments (9)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.alerts/route.tsx (1)
463-463
: Check for consistency in element height.
Setting a fixed height of h-12 might misalign with adjacent elements or text content. Confirm that this is consistent with the application's design guidelines and that it properly accommodates any dynamic content or text overflow.
apps/webapp/app/components/primitives/Table.tsx (8)
2-3
: Imports reorganized for new navigation approach.
The introduction of useNavigate and updated React imports is good. Ensure that all references to Link or anchor tags are replaced or removed if they're no longer in use.
47-47
: Confirm necessity of tabIndex on thead.
Assigning tabIndex={-1} to a header may be unusual unless there's a specific accessibility or keyboard navigation reason. Verify this doesn’t create unintended behavior for screen readers or tab flow.
75-76
: Flexible row-level navigation props.
Allowing to? and onClick? in TableRowProps makes the component more versatile by enabling multiple interaction patterns. This looks good.
80-82
: Use of useNavigate for programmatic navigation.
This is a neat approach for row-based navigation. Keep in mind server-side rendering constraints if used in non-browser contexts.
113-114
: Dynamic tabIndex for disabled rows.
Using tabIndex={disabled ? -1 : 0} is a great practice, ensuring interactive rows are focusable while disabled ones are skipped. This is good for accessibility.
163-163
: tabIndex={-1} for header cells.
This prevents accidentally tabbing into a header cell. It’s typically fine, but confirm that no interactive elements (e.g., tooltips) rely on keyboard focus.
192-192
: Hover and focus styling logic.
The rowHoverStyles merges well with group-hover interactions. Verify that grouping doesn’t override custom focus states if you plan to highlight a selected row or handle user interactions.
244-244
: Solid approach for rendering children directly.
Rendering children without wrapping them in an additional interactive element clarifies the cell’s content model and focuses interactivity at the row level.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/components/runs/v3/RunTag.tsx (1)
Line range hint
6-30
: Consider enhancing keyboard accessibilityGiven this PR's focus on accessibility improvements, consider making the tag component keyboard interactive if it serves as a clickable/interactive element in any context. This would involve:
- Adding appropriate role attribute
- Making it focusable if interactive
- Adding keyboard event handlers
Example enhancement:
export function RunTag({ tag }: { tag: string }) { const tagResult = useMemo(() => splitTag(tag), [tag]); + const isInteractive = false; // determine based on usage context + + const commonProps = isInteractive ? { + role: "button", + tabIndex: 0, + onKeyDown: (e: React.KeyboardEvent) => { + if (e.key === "Enter" || e.key === " ") { + // Handle interaction + } + } + } : {}; if (typeof tagResult === "string") { return ( - <span className="flex h-6 items-stretch"> + <span className="flex h-6 items-stretch" {...commonProps}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/runs/v3/RunTag.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam._index/route.tsx
(1 hunks)
🔇 Additional comments (3)
apps/webapp/app/components/runs/v3/RunTag.tsx (1)
25-27
: LGTM! Consider verifying overflow behavior
The addition of whitespace-nowrap
is a good improvement to prevent awkward text wrapping in tag values. However, ensure that the parent container handles overflow appropriately when tag values are long.
✅ Verification successful
Based on the search results, I can see that the parent components of RunTag have proper overflow handling:
-
The RunTag component is used within containers that have overflow handling classes:
overflow-x-auto
for horizontal scrollingoverflow-y-auto
for vertical scrollingoverflow-hidden
for containing the layout
-
The parent containers also use proper scrollbar styling and grid layouts to manage space effectively.
LGTM! The whitespace-nowrap addition is safe
The addition of whitespace-nowrap
is well-supported by parent containers that properly handle overflow through scrolling mechanisms. The tag values will remain intact without causing layout issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Look for potential overflow handling in parent components
# Search for usage of RunTag to verify parent container styles
rg -l "RunTag" | xargs rg -A 5 -B 5 "RunTag"
# Search for potential overflow handling classes near RunTag usage
rg -l "RunTag" | xargs rg "overflow-\w+|truncate"
Length of output: 38513
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam._index/route.tsx (2)
227-227
: LGTM! Improved spacing between search input and button.
The addition of gap-1
to the flex container provides consistent and appropriate spacing between the search input and the toggle button, improving the visual hierarchy.
Line range hint 227-234
: Consider keyboard navigation implications of autoFocus.
While the autoFocus on the search input aligns with the PR's focus state refinements, ensure this doesn't interfere with users who rely on keyboard navigation to reach other important elements first.
Consider adding a skip link if there are important navigation elements that should be accessible before the search input.
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/webapp/app/components/runs/v3/ScheduleFilters.tsx (1)
106-106
: Ensure consistent spacing between filters.Adding "ml-2" provides extra left margin for the first filter. Check if you should align margins for other filters to maintain a uniform look.
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)
195-199
: Remove the unnecessary Fragment.
Wrapping a single child inside a Fragment is unnecessary. You can safely remove it to address the linter warning and simplify the code.Apply this diff to remove the extra Fragment:
- <> <TableHeaderCell>Compute</TableHeaderCell> - </> + <TableHeaderCell>Compute</TableHeaderCell>🧰 Tools
🪛 Biome (1.9.4)
[error] 195-197: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
apps/webapp/app/components/primitives/Table.tsx (1)
113-117
: Conditional focus for disabled rows.
Using “tabIndex={disabled ? -1 : 0}” properly excludes disabled rows from the tab sequence, improving UX for assistive technologies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/webapp/app/components/primitives/Table.tsx
(7 hunks)apps/webapp/app/components/runs/v3/ScheduleFilters.tsx
(2 hunks)apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam._index/route.tsx
(6 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.alerts/route.tsx
(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.apikeys/route.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.concurrency/route.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments/route.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.environment-variables/route.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.schedules/route.tsx
(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.test/route.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.v3.usage/route.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.test/route.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.environment-variables/route.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.apikeys/route.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam._index/route.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.alerts/route.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
[error] 195-197: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (17)
apps/webapp/app/components/runs/v3/ScheduleFilters.tsx (1)
96-96
: Consider verifying if removing "flex-row" is intentional.
By default, the "flex" class in Tailwind is laid out as a row. Removing "flex-row" may be a no-op. Validate if you intended to maintain row-based layout or switch to a different direction.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.concurrency/route.tsx (1)
83-86
: Streamlined table header structure.
Directly placing TableHeaderCell elements under TableHeader (without TableRow) is consistent with similar refactors in the codebase, reduces nesting, and improves clarity.
apps/webapp/app/routes/_app.orgs.$organizationSlug.v3.usage/route.tsx (1)
210-215
: Consistent header cells for numeric data.
Aligning the numeric data columns (Runs, cost, duration) to the right is a best practice, improving readability and consistency. No issues found.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments/route.tsx (1)
123-153
: Clearer header structure & helpful Status tooltip.
This flattened TableHeader structure, with a descriptive tooltip for deployment statuses, enhances both readability and usability. Ensure that tooltip content remains keyboard-accessible for screen readers.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.schedules/route.tsx (4)
68-68
: New cn utility import.
Importing the cn() utility for conditional classNames is a clean approach to dynamic styling.
214-219
: Dynamic grid layout for schedule table container.
Good use of cn() to conditionally adjust the grid rows based on totalPages, improving layout responsiveness.
221-226
: Conditional pagination styling.
Applying conditional styles for pagination controls only when multiple pages exist is a neat solution to avoid unnecessary space or borders.
388-431
: Informative tooltip explaining schedule types.
The tooltip that distinguishes between DECLARATIVE and IMPERATIVE schedules is well structured, giving users a clear overview. Consider verifying its keyboard accessibility to ensure an inclusive experience.
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (2)
109-130
: Selection column logic looks good.
The checkbox handling for bulk selection is straightforward and appears to integrate well with the existing selection provider. No issues spotted.
250-270
: Tooltip and tagging implementation are well-structured.
The inline documentation and reference links for using tags ensure excellent discoverability.
apps/webapp/app/components/primitives/Table.tsx (7)
2-3
: Good switch to programmatic navigation.
Importing and using useNavigate fosters flexibility if you need to dynamically control routing, rather than relying solely on static links.
75-76
: Enhanced interactivity with “to” and “onClick” props.
The new props and the handling logic allow table rows to be navigable via Enter key and clickable if they have a cell with a “to” prop. This is a solid approach for making rows function like links but still remain accessible.
Also applies to: 80-101, 102-108
192-192
: Hover styles are well-organized.
Utilizing these keyed style variants fosters consistency. No issues here.
234-236
: Optional cursor pointer for actionable cells.
Conditionally applying “cursor-pointer” when “hasAction” is true clarifies user interactivity. This is a clean approach.
316-316
: Absolute positioning for the popover container.
Positioning the popover interaction elements with “absolute” ensures they stay properly aligned. Looks good.
47-47
: Tab index for the header.
Be sure that the negative tab index is intentional. It is useful for programmatic focus, but it will not be keyboard-focusable in normal tab order.
✅ Verification successful
Let me gather more context about the tabIndex usage in the Table component and related components.
Let me try again with corrected shell scripts to gather the necessary context.
Negative tabIndex is intentionally used for proper keyboard navigation
The negative tabIndex in the TableHeader component is part of a well-structured keyboard navigation pattern where:
- The table header (thead) has
tabIndex={-1}
to prevent it from being tab-focusable - Individual table rows have
tabIndex={disabled ? -1 : 0}
to make them focusable when not disabled - Table header cells also have
tabIndex={-1}
to ensure focus remains on interactive elements
This implementation follows accessibility best practices by maintaining a logical tab order through the table's interactive elements while preventing unnecessary focus stops on structural elements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Check the Table component implementation and its context
rg -A 5 -B 5 "tabIndex={-1}" apps/webapp/app/components/primitives/Table.tsx
# Look for other instances of tabIndex usage in the codebase
rg "tabIndex" --type tsx --type jsx
# Check for any accessibility-related comments or documentation
rg -i "accessibility|a11y|keyboard|focus" apps/webapp/app/components/primitives/Table.tsx
Length of output: 1289
Script:
#!/bin/bash
# Check the Table component implementation and its context
rg -A 5 -B 5 'tabIndex="-1"' apps/webapp/app/components/primitives/Table.tsx
# Look for the Table component's header implementation
cat apps/webapp/app/components/primitives/Table.tsx
# Look for other instances of tabIndex usage in the codebase
rg 'tabIndex' .
Length of output: 12713
163-163
: Header cell focus approach.
Adding “tabIndex={-1}” can help with advanced keyboard or screen reader setups, but confirm whether focusing table headers is necessary.
✅ Verification successful
Let me gather more information about the table component and its accessibility implementation to better understand the context of the tabIndex usage.
This will help us understand:
- The context around where tabIndex={-1} is being used
- The accessibility implementation details
- Any keyboard interaction patterns
Let me try another search with fixed regex to better understand the table header implementation:
Based on the search results, I can now provide a final response as we have enough context about the table header implementation:
Table header's tabIndex={-1} is correctly implemented for accessibility
The tabIndex={-1} on the thead element is appropriate as it's part of a well-structured table where:
- The table uses proper semantic elements (thead, th with scope="col")
- Header cells are properly marked with
th
elements - The component includes keyboard navigation support (onKeyDown handlers)
- The implementation follows accessibility patterns with proper ARIA attributes
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Let's examine the Table component implementation and its accessibility patterns
rg -A 5 -B 5 "tabIndex={-1}" apps/webapp/app/components/primitives/Table.tsx
Length of output: 184
Script:
#!/bin/bash
# Look for any ARIA attributes or accessibility-related patterns in the Table component
rg -A 3 -B 3 "aria-|role=" apps/webapp/app/components/primitives/Table.tsx
Length of output: 302
Script:
#!/bin/bash
# Check if there are any keyboard navigation handlers
rg -A 3 -B 3 "onKeyDown|onKeyPress|onKeyUp" apps/webapp/app/components/primitives/Table.tsx
Length of output: 474
Script:
#!/bin/bash
# Search for table header implementation with fixed regex
rg -A 5 -B 5 'tabIndex="-1"' apps/webapp/app/components/primitives/Table.tsx
Length of output: 78
Script:
#!/bin/bash
# Look for TableHeader or Th component implementation
ast-grep --pattern 'const Th = $$$'
Length of output: 3735
Script:
#!/bin/bash
# Check for any header-related accessibility implementations
rg -A 3 -B 3 "th|thead|TableHead" apps/webapp/app/components/primitives/Table.tsx
Length of output: 4078
Focus styles are slightly more subtle
1px border instead of 2px
More subtle focus state for the Task input as it's auto-focused on page load
Tabbing down the table now selects the rows. Enter key to select
Added missing focus-visible states for
Summary by CodeRabbit
New Features
Bug Fixes
Style