test(web): add component test of accessibility page #1349
Conversation
WalkthroughThis pull request introduces comprehensive changes to the Accessibility component across multiple files. The modifications focus on refactoring the component's state management, testing, and functionality. The changes include updating type definitions, restructuring the component's logic, adding a detailed test suite, and improving the testing setup. The primary goal appears to be enhancing the component's robustness, testability, and flexibility in handling model accessibility settings. Changes
Sequence DiagramsequenceDiagram
participant User
participant AccessibilityComponent
participant AccessibilityHook
participant API
User->>AccessibilityComponent: Interact with settings
AccessibilityComponent->>AccessibilityHook: Request update
AccessibilityHook->>API: Send update request
API-->>AccessibilityHook: Return update result
AccessibilityHook-->>AccessibilityComponent: Update component state
AccessibilityComponent->>User: Reflect changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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 (
|
✅ Deploy Preview for reearth-cms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
web/src/components/molecules/Accessibility/index.tsx (1)
Line range hint
186-196: Enhance accessibility for token visibility toggle.The token visibility toggle button lacks proper accessibility attributes for screen readers.
Add ARIA attributes to improve accessibility:
<Icon icon={visible ? "eye" : "eyeInvisible"} + aria-label={visible ? t("Hide token") : t("Show token")} + role="button" + tabIndex={0} onClick={() => { setVisible(prev => !prev); }} + onKeyPress={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + setVisible(prev => !prev); + } + }} />
🧹 Nitpick comments (4)
web/src/test/setup.ts (1)
20-25: Enhance matchMedia mock implementation.The current matchMedia mock is incomplete. Modern web standards require additional methods and properties.
Consider enhancing the mock with additional required properties:
Object.defineProperty(window, "matchMedia", { value: () => ({ + matches: false, + media: "", addListener: () => {}, removeListener: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, }), });web/src/components/organisms/Project/Accessibility/hooks.ts (1)
134-148: Enhance error logging for token regeneration.While the error handling is good, the error logging could be more informative.
Consider enhancing the error logging:
} catch (e) { - console.error(e); + console.error("Failed to regenerate public API token:", e); Notification.error({ message: t("The attempt to re-generate the Public API Token has failed."), });web/src/components/molecules/Accessibility/index.test.tsx (1)
7-175: Add tests for error scenarios and loading states.The test suite is well-structured and comprehensive for happy paths, but could benefit from additional coverage.
Consider adding tests for:
- Error handling when public update fails
- Error handling when token regeneration fails
- Component behavior during loading states
- Accessibility attributes (aria-labels, roles)
Example test structure:
test("Should show error notification when update fails", async () => { const failingUpdate = () => Promise.reject(new Error("Update failed")); render( <Accessibility {...defaultProps} onPublicUpdate={failingUpdate} /> ); // Test implementation }); test("Should disable interactions during loading state", async () => { render( <Accessibility {...defaultProps} updateLoading={true} /> ); // Test implementation });web/src/components/molecules/Accessibility/index.tsx (1)
60-83: Well-implemented change tracking logic!The use of
useRefforchangedModelsanduseCallbackforhandleValuesChangeis an efficient approach to track form changes. The implementation correctly prevents unnecessary re-renders.Consider memoizing the equality check condition to improve readability:
+const isFormUnchanged = useCallback( + (values: FormType) => + initialValues.scope === values.scope && + initialValues.assetPublic === values.assetPublic && + changedModels.current.size === 0, + [initialValues] +); const handleValuesChange = useCallback( (changedValues: Partial<FormType>, values: FormType) => { if (changedValues?.models) { const modelId = Object.keys(changedValues.models)[0]; if (changedModels.current.has(modelId)) { changedModels.current.delete(modelId); } else { changedModels.current.set(modelId, changedValues.models[modelId]); } } - if ( - initialValues.scope === values.scope && - initialValues.assetPublic === values.assetPublic && - changedModels.current.size === 0 - ) { + if (isFormUnchanged(values)) { setIsSaveDisabled(true); } else { setIsSaveDisabled(false); } }, - [initialValues], + [isFormUnchanged], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/components/molecules/Accessibility/index.test.tsx(1 hunks)web/src/components/molecules/Accessibility/index.tsx(5 hunks)web/src/components/organisms/Project/Accessibility/hooks.ts(5 hunks)web/src/components/organisms/Project/Accessibility/index.tsx(1 hunks)web/src/test/setup.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: ci-web / ci
- GitHub Check: Redirect rules - reearth-cms
- GitHub Check: Header rules - reearth-cms
- GitHub Check: Pages changed - reearth-cms
🔇 Additional comments (3)
web/src/components/organisms/Project/Accessibility/index.tsx (1)
7-30: LGTM! Clean refactoring of component props.The refactoring improves component maintainability by:
- Removing form instance dependency in favor of direct value management
- Simplifying prop interface with AccessibilityMolecule
web/src/test/setup.ts (1)
27-30: LGTM! Proper preservation of getComputedStyle functionality.The implementation correctly preserves the original getComputedStyle behavior while allowing test overrides.
web/src/components/molecules/Accessibility/index.tsx (1)
21-23: Consider security implications of exposing API endpoints.The
endpointfield inModelDataTypeexposes API URLs in the UI. While this improves developer experience, ensure that these endpoints don't expose sensitive information or internal infrastructure details.✅ Verification successful
Endpoint exposure is secure and follows best practices
The exposure of API endpoints is an intentional design choice that follows security best practices:
- Endpoints only expose validated model keys (alphanumeric +
_-)- Access is protected by scope controls and token authentication
- No sensitive information is exposed in the URLs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential sensitive information in API endpoints rg -i "(internal|private|secret|password|key|token|credential)" --type tsLength of output: 87675
…st-web/accessibility
Overview
This PR adds the test for component test of accessibility page.
I refactored and optimized components to run the test.
Memo
E2E test has been passed.
Summary by CodeRabbit
Tests
Refactor
Chores