feat: adding aria-live region on error #66
Conversation
## Walkthrough
The changes across the project involve enhancing accessibility in UI components, specifically the `TextInput` component, by adding live region functionality and ARIA attributes. The project's build and lint configurations have been updated, with adjustments to asset size constraints and the centralization of external dependencies. ESLint rules have been modified for better code quality, and several configuration files have been updated to disable the "no-undef" rule, indicating a shift in linting strategy.
## Changes
| File(s) | Change Summary |
| --- | --- |
| `packages/ui-components/src/common/constants.ts` | Removed export of `RAW_CLASSNAME` constant. |
| `packages/ui-components/src/components/TextInput/TextInput.tsx` | Added `LiveRegion` import, `liveErrorMessage` variable, `aria-invalid` attribute, and `LiveRegion` component for error handling. |
| `packages/ui-components/src/components/TextInput/__tests__/TextInput.test.tsx` | Updated tests for class name expectations and added accessibility-related test cases. |
| `packages/ui-components/src/components/TextInput/utilities.ts` | Removed `RAW_CLASSNAME` usage, updated function signatures and logic for class assignment based on `raw` parameter. |
| `bundlemon.config.cjs` | Updated asset size constraints and added `maxPercentIncrease` for `index.js` and `style.css`. |
| `packages/ui-components/vite.config.ts` | Added `react-dom` and `react-dom/server` to external dependencies. |
| `.eslintrc.cjs` | Added root definition, extended configurations, ignore patterns, TypeScript parser, new plugins, and modified rules. |
| `configuration/eslint-rules/*.cjs` | Disabled "no-undef" eslint rule in various ESLint rule configuration files. |
| `configuration/lint-staged.config.cjs` | Added comment to disable "no-undef" eslint rule. |
| `configuration/vite.common.js` | Added `externalDependencies` array with project dependencies. |
| `packages/bundlesize/vite.config.ts` | Imported `externalDependencies` from common configuration to replace hardcoded dependencies. |TipsChat with CodeRabbit Bot (
|
BundleMonFiles updated (1)
Unchanged files (2)
Total files change +484B +5.54% Final result: ✅ View report in BundleMon website ➡️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (4)
- packages/ui-components/src/common/constants.ts (1 hunks)
- packages/ui-components/src/components/TextInput/TextInput.tsx (4 hunks)
- packages/ui-components/src/components/TextInput/tests/TextInput.test.tsx (2 hunks)
- packages/ui-components/src/components/TextInput/utilities.ts (4 hunks)
Files skipped from review due to trivial changes (1)
- packages/ui-components/src/common/constants.ts
Additional comments: 11
packages/ui-components/src/components/TextInput/__tests__/TextInput.test.tsx (3)
44-48: The test case correctly checks for an empty class name for a raw text input with no styling. This aligns with the refactoring changes mentioned in the summary.
79-95: This test case verifies the presence of ARIA attributes when an error message is rendered, which is crucial for accessibility. It ensures that
aria-invalidis set to true and thataria-describedbycontains the correct references. This is a good practice for accessibility testing.97-114: The test case for the live region update is well-implemented. It checks the
aria-liveattribute and the content of the live region before and after a specified delay. This ensures that the live region is updated correctly, which is important for screen reader users to receive timely feedback.packages/ui-components/src/components/TextInput/TextInput.tsx (4)
2-4: The import of
LiveRegionis correctly added to support the new accessibility features. Ensure that theLiveRegioncomponent is properly tested in the test suite.27-28: The
liveErrorMessagevariable concatenates thenameandhelperText, which is a good approach for creating descriptive error messages for screen readers. However, ensure that thenameis user-friendly and appropriate for being read aloud by assistive technologies.58-58: The use of the
aria-invalidattribute when there is an error is a good practice for accessibility, as it helps users of assistive technologies identify fields with errors.77-79: The
LiveRegioncomponent is correctly implemented to announce theliveErrorMessage. It's important to ensure that the delay and politeness level are appropriate and tested for the user experience.packages/ui-components/src/components/TextInput/utilities.ts (4)
66-75: The
getTextInputHelperTextClassesfunction now correctly handles therawparameter to conditionally returnundefinedifrawis true. This change aligns with the pull request's goal to refactor the code to control styling through a boolean parameter.87-93: The
getTextInputClassesfunction has been updated to include therawparameter and conditionally assignundefinedtowrapper. This change is consistent with the refactoring goal mentioned in the pull request summary.87-99: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [95-108]
The
getTextInputClassesfunction now uses therawparameter to conditionally assignundefinedtoinputandwrapper. This is a good use of theclsxlibrary to manage conditional class names and supports the refactoring effort to remove the dependency on theRAW_CLASSNAME.
- 110-114: The
topLabelvariable is correctly set toundefinedwhenrawis true, and thehelperTextvariable is updated to callgetTextInputHelperTextClasseswith the newerrorandrawparameters. This maintains consistency with the new approach to handling styles.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- bundlemon.config.cjs (1 hunks)
- packages/ui-components/vite.config.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/ui-components/vite.config.ts
Additional comments: 2
bundlemon.config.cjs (2)
9-13:
The maximum size for "assets/index.js" has been reduced significantly from "60kb" to "10kb". Ensure that this change is intentional and that the current size of "assets/index.js" complies with the new limit to avoid build failures.16-18:
A maximum percent increase constraint of 5% has been added for "assets/style.css". Verify that this new constraint aligns with the project's performance goals and that the current size of "assets/style.css" is within acceptable limits.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (9)
- .eslintrc.cjs (1 hunks)
- bundlemon.config.cjs (2 hunks)
- configuration/eslint-rules/best-practices.cjs (1 hunks)
- configuration/eslint-rules/possible-errors.cjs (1 hunks)
- configuration/eslint-rules/variables.cjs (1 hunks)
- configuration/lint-staged.config.cjs (1 hunks)
- configuration/vite.common.js (1 hunks)
- packages/bundlesize/vite.config.ts (2 hunks)
- packages/ui-components/vite.config.ts (2 hunks)
Files skipped from review due to trivial changes (6)
- configuration/eslint-rules/best-practices.cjs
- configuration/eslint-rules/possible-errors.cjs
- configuration/eslint-rules/variables.cjs
- configuration/lint-staged.config.cjs
- packages/bundlesize/vite.config.ts
- packages/ui-components/vite.config.ts
Additional comments: 5
configuration/vite.common.js (1)
- 1-9: The addition of the
externalDependenciesarray centralizes the management of external dependencies for the build process, which is a good practice for maintainability and consistency across different configuration files..eslintrc.cjs (1)
- 1-25: - The ESLint configuration has been updated to include new plugins and rules, which should help maintain code quality and consistency. Ensure that the team is aware of these changes and that they understand the new rules, especially the implications of disabling the
@typescript-eslint/no-explicit-anyrule, which could lead to less type-safe code.
- Verify that the
ignorePatternsare correctly ignoring the intended files and directories, and that no necessary files are being excluded from linting.- Confirm that the new plugins "react-refresh" and "simple-import-sort" are installed and properly configured in the project's
package.json.- The addition of custom rule files (
best-practices.cjs,possible-errors.cjs, andvariables.cjs) suggests a move towards a more tailored linting setup. Ensure that these files are present at the specified paths and contain the expected rule configurations.- The
rootproperty correctly indicates that this is the root ESLint configuration, which is important for projects with nested configurations.bundlemon.config.cjs (3)
1-1: The comment to disable the "no-undef" ESLint rule is present, but it's important to ensure that this rule is disabled for a valid reason and that it does not lead to unidentified global variables being used in the codebase.
13-15: The
maxSizefor "assets/index.js" has been significantly reduced from "60kb" to "10kb". This is a substantial decrease and could potentially break the build if the current size exceeds this new limit. Verify that the current size of "assets/index.js" is within this new constraint and that themaxPercentIncreaseof 5 is appropriate.18-20: Similarly, the
maxSizefor "assets/style.css" has been set to "4kb" with amaxPercentIncreaseof 5. Ensure that the current size of "assets/style.css" complies with this new limit and that themaxPercentIncreaseis suitable for the project's needs.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- bundlemon.config.cjs (2 hunks)
Additional comments: 2
bundlemon.config.cjs (2)
1-1: The comment to disable the "no-undef" ESLint rule is correctly placed at the top of the file. However, ensure that this rule is intentionally disabled and that it does not lead to any unintended consequences in the codebase.
13-14: The maximum size for "assets/index.js" has been reduced significantly from "60kb" to "10kb". Confirm that this new size constraint is realistic and that the current size of "assets/index.js" complies with this updated limit to avoid build failures.
Summary by CodeRabbit
New Features
TextInputcomponent with live error messaging andaria-invalidattribute for improved accessibility.Refactor
RAW_CLASSNAMEconstant and related class assignment logic.Style
bundlemon.config.cjsfor better performance monitoring.Tests
TextInputcomponent to validate accessibility features.Chores