-
-
Notifications
You must be signed in to change notification settings - Fork 53
Updated Table props for the docs #771
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
Conversation
…or better accessibility and updated radui package to v0.028
|
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
docs/app/docs/components/accordion/docs/codeUsage.jsOops! Something went wrong! :( ESLint: 8.56.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/docs/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request introduces consistent changes across multiple documentation files for various components. The primary modification involves replacing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
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 (
|
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: 1
🧹 Nitpick comments (1)
docs/app/docs/components/visually-hidden/page.js (1)
21-21: Consider enhancing the prop description for clarity.The addition of the 'id' field is good, but the description "renders the children in desired element" could be more specific.
Consider updating the description to be more descriptive, for example:
-{prop: 'asChild', type: 'boolean', default: 'false', description: 'renders the children in desired element', id: 'asChild'}, +{prop: 'asChild', type: 'boolean', default: 'false', description: 'When true, allows rendering the hidden content within a custom element instead of the default span', id: 'asChild'},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
docs/app/docs/components/accordion/docs/codeUsage.js(2 hunks)docs/app/docs/components/aspect-ratio/page.js(1 hunks)docs/app/docs/components/avatar/page.js(1 hunks)docs/app/docs/components/visually-hidden/page.js(1 hunks)docs/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (6)
docs/app/docs/components/aspect-ratio/page.js (1)
13-16: LGTM! Changes align with accessibility improvementsThe replacement of 'key' with 'id' in both columns and data arrays is consistent with the PR objectives and improves accessibility.
Also applies to: 20-20
docs/app/docs/components/avatar/page.js (1)
14-17: LGTM! Consistent implementation of accessibility improvementsThe changes follow the same pattern of replacing 'key' with 'id', maintaining consistency across the documentation.
Also applies to: 21-22
docs/app/docs/components/accordion/docs/codeUsage.js (2)
29-33: LGTM! Keyboard shortcuts columns updated correctlyThe column structure has been updated to use 'id' instead of 'key', improving accessibility.
94-95: Address TODO comments in keyboard shortcutsThe Home and End keyboard shortcuts are marked with [TODO]. These implementations should be completed for full keyboard navigation support.
Would you like me to help implement the Home and End keyboard shortcut functionality?
Also applies to: 101-102
docs/package.json (1)
13-13: Verify @radui/ui package versionThe package has been updated to version ^0.0.28. Let's verify this is the latest stable version.
✅ Verification successful
@radui/ui package version is up to date
The package version ^0.0.28 specified in package.json matches the latest version available in the npm registry.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check latest version of @radui/ui package # Note: Using npm view as it's more reliable for scoped packages npm view @radui/ui versions --json | jq -r '.[-1]'Length of output: 59
docs/app/docs/components/visually-hidden/page.js (1)
14-17: LGTM! Column structure follows the new accessibility standards.The change from 'key' to 'id' in column definitions aligns with the PR objectives to enhance accessibility.
| {name: 'Prop', id: 'prop'}, | ||
| {name: 'Type', id: 'type'}, | ||
| {name: 'Default', id: 'default'}, | ||
| {name: 'Description', id: 'description'}, | ||
| ]; | ||
|
|
||
| const data = [ | ||
| {prop: 'asChild', type: 'boolean', default: 'false', description: 'renders the children in desired element'}, | ||
| {prop: 'asChild', type: 'boolean', default: 'false', description: 'renders the children in desired element', id: 'asChild'}, |
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.
💡 Codebase verification
Inconsistencies found in component documentation structure
Found inconsistencies that should be addressed:
- The
avatar-groupcomponent useskeyinstead ofidin column definitions, while other components useid - Data entries have inconsistent field presence:
- Some components include
idfield (visually-hidden, avatar, aspect-ratio) avatar-grouplacksidfield but includes additionalvaluesfield
- Some components include
Consider standardizing these patterns across all component documentation files for better maintainability.
🔗 Analysis chain
Verify consistent implementation across other component docs.
Let's ensure this pattern is consistently applied across other component documentation files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistency in column definitions across component docs
echo "Checking column definitions in component documentation files..."
rg -U --multiline 'const\s+columns\s*=\s*\[\s*{[^}]*}\s*\]' docs/app/docs/components/*/page.js
# Check for proper id usage in data arrays
echo "Checking data array structures..."
rg -U --multiline 'const\s+data\s*=\s*\[\s*{[^}]*}\s*\]' docs/app/docs/components/*/page.js
Length of output: 3360
Script:
#!/bin/bash
# Check column definitions with fixed regex and context
echo "Checking column definitions..."
rg -U --multiline -B1 -A3 'const columns = \[' docs/app/docs/components/*/page.js
echo -e "\nChecking data array structures..."
rg -U --multiline -B1 -A3 'const data = \[' docs/app/docs/components/*/page.js
Length of output: 6235
Update keyboard shortcut documentation to use 'id' instead of 'key' for better accessibility and updated radui package to v0.028
This pr fixes #761
Summary by CodeRabbit
Documentation
keywithidDependency Update
@radui/uipackage from version 0.0.27 to 0.0.28