Visually hidden docs #691
Visually hidden docs #691
Conversation
|
Caution Review failedThe pull request is closed. 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/components/navigation/Navigation.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. WalkthroughThis pull request introduces documentation for the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/app/docs/components/visually-hidden/page.js (1)
26-42: Suggestion: Demonstrate the visible result for testing.Embedding a short note alongside the visually hidden text could help illustrate how it remains hidden visually but still accessible to screen readers, enabling new users to confirm the behavior in dev tools or usage demos.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/app/docs/components/visually-hidden/docs/codeUsage.js(1 hunks)docs/app/docs/components/visually-hidden/page.js(1 hunks)docs/components/navigation/Navigation.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/app/docs/components/visually-hidden/docs/codeUsage.js
🔇 Additional comments (3)
docs/app/docs/components/visually-hidden/page.js (2)
1-7: Good addition of metadata and naming convention.
The PAGE_NAME constant and the metadata export align well with the existing documentation standards. No issues spotted.
11-23: Comprehensive table structure.
Your definition of columns and data for the prop table is clear and follows best practices for a compact, well-documented layout. This is helpful for future expansion if more props are added.
docs/components/navigation/Navigation.js (1)
157-160: VisuallyHidden navigation link added successfully.
The path and title match the newly added documentation route. Verify that it is appropriately reachable within the docs site navigation and appears in the correct "Components" list position.
✅ Verification successful
Navigation link for VisuallyHidden component is properly integrated
The verification confirms that:
- The component documentation exists at the correct path
/docs/components/visually-hidden - The navigation entry is properly added with the correct title "VisuallyHidden"
- The component is fully documented with proper examples, code usage, and implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Confirm that the hyperlink to /docs/components/visually-hidden is properly rendered.
rg 'VisuallyHidden' -A 3
Length of output: 7537
| import SEO from "../../docsIndex" | ||
| export const metadata = SEO.getMetadata(PAGE_NAME) | ||
|
|
||
| import codeUsage from "./docs/codeUsage" |
There was a problem hiding this comment.
💡 Codebase verification
The import path needs to be corrected
The current import path "./docs/codeUsage" is incorrect. Based on the directory structure and file content verification:
- The file exists at
docs/app/docs/components/visually-hidden/docs/codeUsage.js - It exports a default object containing code examples
- The import statement should be
"./docs/codeUsage"since the importing file (page.js) is in the same directory as thedocsfolder
🔗 Analysis chain
Suggest verifying the import path.
Kindly double-check that "./docs/codeUsage" is the correct relative path for the codeUsage import and that the file indeed exports what you expect.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Validate that codeUsage is exported from docs/app/docs/components/visually-hidden/docs/codeUsage.js
rg --context 3 'export .* codeUsage'
Length of output: 38
Script:
#!/bin/bash
# Let's first check if the file exists at the expected location
fd "codeUsage" --extension js --extension ts --extension tsx
# Let's also check for any file that might export codeUsage using ast-grep
ast-grep --pattern 'export $_ codeUsage'
# Let's also check the directory structure to understand the context better
ls -R docs/app/docs/components/visually-hidden/
Length of output: 1236
Script:
#!/bin/bash
# Let's check the content of the codeUsage.js file to verify the export
cat docs/app/docs/components/visually-hidden/docs/codeUsage.js
# Let's also check the actual import statement in page.js
cat docs/app/docs/components/visually-hidden/page.js
Length of output: 1872
| </VisuallyHidden> | ||
| )` | ||
| }, | ||
| scss: { |
There was a problem hiding this comment.
If we do this via js as an inline style, we wont need the CSS, this wont work without CSS, thoughts?
There was a problem hiding this comment.
The component was made this way although it has a line of styling but inline styling might have conflicts with customclass styling. Not tested I might be wrong too.
There was a problem hiding this comment.
if people wanna inline style and override, that's fine - but out of the box this should work without CSS
Coz we are a headless library first, without CSS - this component will be visible even though we call it visually hidden :D
GoldGroove06
force-pushed
the
visually-hidden-docs
branch
from
f12b246 to
6afc571
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/app/docs/components/visually-hidden/page.js (5)
3-4: Remove unnecessary empty line between importsConsider removing the extra empty line to maintain consistent spacing between imports.
20-23: Enhance API documentation completenessThe current API documentation could be improved in several ways:
- The description for
asChildcould be more detailed, explaining when and why to use it- Consider documenting additional common props (e.g., className, style) if supported
- Add examples of how
asChildaffects rendering with different HTML elementsExample enhancement for the data array:
const data = [ - {prop: 'asChild', type: 'boolean', default: 'false', description: 'renders the children in desired element'}, + { + prop: 'asChild', + type: 'boolean', + default: 'false', + description: 'When true, the component will render its children using the specified element type instead of the default span. Useful for maintaining semantic HTML while applying visual hiding.' + }, + { + prop: 'className', + type: 'string', + default: 'undefined', + description: 'Additional CSS classes to apply to the component' + } ];
29-33: Enhance the example and clean up markup
- The empty div wrapper around VisuallyHidden seems unnecessary
- Consider adding more practical examples that demonstrate real-world use cases
-<div> -<VisuallyHidden asChild> - <span>This is a visually hidden text</span> -</VisuallyHidden> -</div> +<VisuallyHidden asChild> + <h2>Skip to main content</h2> +</VisuallyHidden>Consider adding more examples like:
- Skip navigation links
- Form labels that need to be hidden
- Screen reader announcements
37-39: Remove unnecessary div wrapperThe div wrapper around Documentation.Table appears to be unnecessary and adds no semantic value.
-<div > <Documentation.Table columns={columns} data={data} /> -</div>
27-27: Enhance accessibility documentationSince this is an accessibility-focused component, consider expanding the description to include:
- WCAG guidelines this component helps satisfy
- Common accessibility pitfalls it helps avoid
- Best practices for implementation
- Links to relevant accessibility resources
Example enhancement:
-description={`Use these helpers to visually hide elements but keep them accessible to assistive technologies.`} +description={`Use these helpers to visually hide elements while maintaining accessibility for screen readers and other assistive technologies. This component follows WCAG techniques for visually hidden content (C7) and helps maintain an inclusive user experience while managing visual complexity.`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/app/docs/components/visually-hidden/docs/codeUsage.js(1 hunks)docs/app/docs/components/visually-hidden/page.js(1 hunks)docs/components/navigation/Navigation.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/app/docs/components/visually-hidden/docs/codeUsage.js
- docs/components/navigation/Navigation.js
🔇 Additional comments (1)
docs/app/docs/components/visually-hidden/page.js (1)
9-9: The import path needs to be corrected
The import path "./docs/codeUsage" needs to be verified.
kotAPI
added
the
automerge
#689 Adding visually hidden component to docs in the website with example and api documentation. Need suggestion on how to show the hidden component in example.
Summary by CodeRabbit
New Features
Documentation