cheton commented 3 months ago

PR Type

Enhancement, Tests

Description

Added role="region" to AccordionContent for improved accessibility.
Added comprehensive unit tests for Accordion, Textarea, and VisuallyHidden components.
Tests cover rendering, attribute handling, event handling, and visual styles.

Changes walkthrough 📝

Relevant files

Enhancement

AccordionContent.js

Add `role="region"` to AccordionContent for accessibility

packages/react/src/accordion/AccordionContent.js - Added `role="region"` to improve accessibility.

+1/-0

Tests

Accordion.test.js `Add unit tests for Accordion component` packages/react/src/accordion/__tests__/Accordion.test.js Added unit tests for the Accordion component. Tested rendering, toggling visibility, handling multiple items, and disabled state.	+189/-0
Textarea.test.js `Add unit tests for Textarea component` packages/react/src/textarea/__tests__/Textarea.test.js Added unit tests for the Textarea component. Tested rendering, disabled, error, readonly, required attributes, onChange function, and border color.	+135/-0
VisuallyHidden.test.js `Add unit tests for VisuallyHidden component` packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js Added unit tests for the VisuallyHidden component. Tested rendering and visual hiding of content.	+37/-0

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codesandbox[bot] commented 3 months ago

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders
Open Preview

codesandbox-ci[bot] commented 3 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codesandbox-ci[bot] commented 3 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

trendmicro-frontend-bot commented 3 months ago

Tonic UI Demo

2024-06-14 15:48:00 +0000 The PR #870 (aae0fea4798ff6f1088fe2fda342a5e792b0baa3) has been deployed to https://trendmicro-frontend.github.io/tonic-ui-demo/react/pr-870/

codesandbox-ci[bot] commented 3 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codesandbox-ci[bot] commented 3 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codesandbox-ci[bot] commented 3 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codesandbox-ci[bot] commented 3 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

cheton commented 2 months ago

/review --pr_reviewer.inline_code_comments=true --pr_reviewer.require_score_review=true

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/trendmicro-frontend/tonic-ui/commit/aae0fea4798ff6f1088fe2fda342a5e792b0baa3)

⏱️ Estimated effort to review [1-5]	3
🏅 Score	92
🧪 Relevant tests	Yes
🔒 Security concerns	No
⚡ Key issues to review	None

cheton commented 2 months ago

/improve

cheton commented 2 months ago

/add_docs

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Documentation 📚

Here is a list of the files that were modified in the PR, with docstring for each altered code component:

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Code Suggestions ✨

Textarea.test.js

arrow_function (function) ^[+99/-0]

Component signature:
```javascript const arrow_function = () => { ``` Docstring:
```javascript /** * Test suite for the Textarea component. * * This function contains multiple test cases to ensure the Textarea component * renders correctly under various conditions and interactions. * * @function */ ```

VisuallyHidden.test.js

arrow_function (function) ^[+32/-0]

Component signature:
```javascript const arrow_function = () => { ``` Docstring:
```javascript /** * Test suite for the VisuallyHidden component. * * @function * @name arrow_function * * @description * This function contains tests for the VisuallyHidden component to ensure it renders correctly and that its content is visually hidden but still present in the document. * * @example * describe('VisuallyHidden', () => { * it('should render correctly', async () => { * const { container } = render( * Hidden Content * ); * * expect(container).toMatchSnapshot(); * * await testA11y(container); * }); * * it('should render the content but visually hidden', () => { * const { getByText } = render( * Hidden Content * ); * const content = getByText('Hidden Content'); * * // Check that the content is in the document * expect(content).toBeInTheDocument(); * * // Check that the content is visually hidden * const styles = window.getComputedStyle(content); * expect(styles.position).toBe('absolute'); * expect(styles.width).toBe('1px'); * expect(styles.height).toBe('1px'); * expect(styles.padding).toBe('0px'); * expect(styles.border).toBe('0px'); * expect(styles.overflow).toBe('hidden'); * expect(styles.clipPath).toBe('inset(50%)'); * expect(styles.whiteSpace).toBe('nowrap'); * }); * }); */ ```

Category	Suggestion	Score
Enhancement	Add an assertion to check the initial value of the textarea before typing ___ In the test for the `onChange` function, add an assertion to check the initial value of the textarea before typing to ensure the test covers the initial state as well. [packages/react/src/textarea/__tests__/Textarea.test.js [75-77]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR75-R77) ```diff const textarea = screen.getByTestId('textarea'); +expect(textarea).toHaveDisplayValue(''); await user.type(textarea, 'hello'); expect(onChange).toHaveBeenCalled(); ``` - [x] Apply this suggestion Suggestion importance[1-10]: 7 Why: This is a valid enhancement. Adding an assertion to check the initial value of the textarea ensures that the test covers the initial state, which is important for verifying that the textarea starts empty before any user interaction.	7
Compatibility	Update the assertion to use `clip` instead of `clipPath` for better browser compatibility ___ The `expect(styles.clipPath).toBe('inset(50%)');` assertion on line 34 should be updated to `expect(styles.clip).toBe('rect(1px, 1px, 1px, 1px)');` for better compatibility with older browsers that do not support `clipPath`. [packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js [34]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-0e247f86dd40408f95235c167d00efc8abc097e6704a3d6f2ff991246f1202b7R34-R34) ```diff -expect(styles.clipPath).toBe('inset(50%)'); +expect(styles.clip).toBe('rect(1px, 1px, 1px, 1px)'); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: This suggestion is valid as using `clip` instead of `clipPath` can enhance compatibility with older browsers. It's a good practice to ensure that tests and styles are robust across different environments.	6

codesandbox-ci[bot] commented 2 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.69%. Comparing base (beee3ee) to head (aae0fea).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## v2 #870 +/- ## ========================================== + Coverage 71.92% 73.69% +1.77% ========================================== Files 375 375 Lines 6140 6140 ========================================== + Hits 4416 4525 +109 + Misses 1724 1615 -109 ``` | [Flag](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | Coverage Δ | | |---|---|---| | [codemod](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `74.28% <ø> (ø)` | | | [react](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `71.74% <ø> (+2.03%)` | :arrow_up: | | [react-base](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `100.00% <ø> (ø)` | | | [react-hooks](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `87.59% <ø> (ø)` | | | [styled-system](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `93.42% <ø> (ø)` | | | [theme](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `100.00% <ø> (ø)` | | | [utils](https://app.codecov.io/gh/trendmicro-frontend/tonic-ui/pull/870/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend) | `70.28% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trendmicro-frontend#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codiumai-pr-agent-pro[bot] commented 2 months ago

Persistent review updated to latest commit https://github.com/trendmicro-frontend/tonic-ui/commit/abbd8adb5df402143cf7e55ead001fe8d40b99a0

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Accessibility	Add accessibility check for the `Textarea` component with the `disabled` attribute ___ The `testA11y` function call should include a check for the `Textarea` component with the `disabled` attribute to ensure accessibility compliance. [packages/react/src/textarea/__tests__/Textarea.test.js [38-40]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR38-R40) ```diff -render( +const { container } = render( ); +await testA11y(container); ``` - [x] Apply this suggestion <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 8</summary> Why: This is a valuable suggestion as it ensures that accessibility tests are performed on components with different states, which is crucial for compliance and inclusivity. </details></details></td><td align=center>8 </td></tr><tr><td> <details><summary>Add accessibility check for the <code>Textarea</code> component with the <code>error</code> attribute</summary> ___ The <code>testA11y</code> function call should include a check for the <code>Textarea</code> component with the <br><code>error</code> attribute to ensure accessibility compliance. [packages/react/src/textarea/__tests__/Textarea.test.js [46-48]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR46-R48) ```diff -render( +const { container } = render( <Textarea data-testid="textarea" error /> ); +await testA11y(container); ``` - [x] Apply this suggestion <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 8</summary> Why: Similar to the previous suggestion, ensuring accessibility for components in an error state is important for user inclusivity and compliance with accessibility standards. </details></details></td><td align=center>8 </td></tr><tr><td> <details><summary>Add accessibility check for the <code>Textarea</code> component with the <code>readOnly</code> attribute</summary> ___ The <code>testA11y</code> function call should include a check for the <code>Textarea</code> component with the <br><code>readOnly</code> attribute to ensure accessibility compliance. [packages/react/src/textarea/__tests__/Textarea.test.js [54-56]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR54-R56) ```diff -render( +const { container } = render( <Textarea data-testid="textarea" readOnly /> ); +await testA11y(container); ``` - [x] Apply this suggestion <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 8</summary> Why: This suggestion is important as it addresses the need to verify accessibility compliance for read-only states of components, ensuring that all users can interact with the UI as intended. </details></details></td><td align=center>8 </td></tr><tr><td rowspan=1><strong>Possible issue</strong></td> <td> <details><summary>Add a check to ensure the <code>onChange</code> function is called with the correct value</summary> ___ The test for the <code>onChange</code> function should include a check to ensure that the <code>onChange</code> <br>function is called with the correct value. This will make the test more robust and ensure <br>that the function behaves as expected. [packages/react/src/textarea/__tests__/Textarea.test.js [77-78]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR77-R78) ```diff await user.type(textarea, 'hello'); -expect(onChange).toHaveBeenCalled(); +expect(onChange).toHaveBeenCalledWith(expect.anything()); ``` - [x] Apply this suggestion <!-- /improve --apply_suggestion=3 --> <details><summary>Suggestion importance[1-10]: 7</summary> Why: The suggestion correctly identifies an improvement in the test to verify not just the call but the value passed to the `onChange` function, enhancing the test's effectiveness. </details></details></td><td align=center>7 </td></tr></tr></tbody></table> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>CI Failure Feedback 🧐</h2> <table><tr><td> Action: review</td></tr> <tr><td> Failed stage: [AI Code Reviewer](https://github.com/trendmicro-frontend/tonic-ui/actions/runs/9517351637/job/26235745044) [❌] </td></tr> <tr><td> Failure summary: The action failed due to an HTTP error while attempting to post a review comment on the pull <br>request. The specific error encountered was:<br> <li> <code>Unprocessable Entity</code>: "Pull request review thread path is invalid and Pull request review thread <br>diff hunk can't be blank"<br> This indicates that there was an issue with the path or the diff hunk in <br>the review comment being posted.<br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: ##[group]Operating System 2: Ubuntu ... 115: with: 116: GITHUB_TOKEN: * 117: AZURE_OPENAI_API_KEY: * 118: AZURE_OPENAI_API_VERSION: 2024-05-01-preview 119: AZURE_OPENAI_DEPLOYMENT: GPT4o 120: AZURE_OPENAI_ENDPOINT: * 121: exclude: /yarn.lock, /package.json, /README.md 122: ##[endgroup] 123: Error: RequestError [HttpError]: Unprocessable Entity: "Pull request review thread path is invalid and Pull request review thread diff hunk can't be blank" ... 149: 'x-ratelimit-remaining': '4997', 150: 'x-ratelimit-reset': '1718377145', 151: 'x-ratelimit-resource': 'core', 152: 'x-ratelimit-used': '3', 153: 'x-xss-protection': '0' 154: }, 155: data: { 156: message: 'Unprocessable Entity', 157: errors: [Array], ... 163: method: 'POST', 164: url: 'https://api.github.com/repos/trendmicro-frontend/tonic-ui/pulls/870/reviews', 165: headers: { 166: accept: 'application/vnd.github.v3+json', 167: 'user-agent': 'octokit-rest.js/19.0.7 octokit-core.js/4.2.0 Node.js/16.20.2 (linux; x64)', 168: authorization: 'token [REDACTED]', 169: 'content-type': 'application/json; charset=utf-8' 170: }, 171: body: '{"comments":[{"body":"`uses: actions/checkout@v4` should be verified for the latest stable release, as the latest stable version is `v2`. Ensure that `v4` exists and is stable or switch to `v2`.","path":".github/workflows/ai-code-reviewer.yml","line":14},{"body":"Avoid long comments within the `with` section. Consider moving the explanation about `GITHUB_TOKEN` to the `README` or documentation file.","path":".github/workflows/ai-code-reviewer.yml","line":19},{"body":"Use a more specific error message to make it clear why the error is occurring. Currently, it suggests checking the README.md, but providing more context or details about the missing codemod would be helpful.","path":"packages/codemod/__tests__/codemod.test.js","line":57},{"body":"Consider checking if the ignorePattern is an array and also handle the case where it\'s a single string. This would make the test more robust.","path":"packages/codemod/__tests__/codemod.test.js","line":128},{"body":"The default value for the `dry` option is redundant as it\'s already `false` by default for boolean types. Consider removing it.","path":"packages/codemod/codemod-cli.js","line":25},{"body":"The default value for the `print` option is redundant as it\'s already `false` by default for boolean types. Consider removing it.","path":"packages/codemod/codemod-cli.js","line":44},{"body":"`/node_modules/` can be extracted as a constant for better readability and reusability. Consider defining it at the top of the file.","path":"packages/codemod/codemod-cli.js","line":35},{"body":"Consider enumerating the valid values for the `parser` option to prevent invalid input from users.","path":"packages/codemod/codemod-cli.js","line":39},{"body":"Consider using a more descriptive default value for the `verbose` option, like `1`, to provide meaningful feedback without being overly verbose.","path":"packages/codemod/codemod-cli.js","line":49},{"body":"Consider adding a more descriptive error message or handling specific error types to provide better context for troubleshooting.","path":"packages/codemod/codemod-main.js","line":37},{"body":"You might want to add a comment here about the issue linking itself rather than explaining it inline to maintain code cleanliness.","path":"packages/codemod/codemod-main.js","line":41},{"body":"Instead of checking for undefined with `flags.verbose !== undefined`, consider using a more explicit check method like `flags.hasOwnProperty(\'verbose\')` for clarity.","path":"packages/codemod/codemod-main.js","line":69},{"body":"The spread operator `{...props}` might unintentionally overwrite attributes like `as` and `size` for the `Icon` component if such properties are passed in via `props`. Consider explicitly listing props that are safe to spread or making sure that these specific attributes remain unchanged.","path":"packages/codemod/src/react/v2.0.0/__tests__/__fixtures__/expected.js","line":10},{"body":"Using the rest operator (`...props`) in `<Icon icon=\\"alert\\" {...props} />` might lead to unexpected behavior or potential vulnerability if not properly sanitized. Ensure that `props` is sanitized or controlled to prevent passing unintended properties.","path":"packages/codemod/src/react/v2.0.0/__tests__/__fixtures__/source.js","line":9},{"body":"The optional chaining (`?.`) on `attribute.name` is unnecessary unless `attribute.name` can be `undefined`. If `attribute.name` can never be `undefined`, remove the optional chaining for cleaner and more efficient code. If it can be, ensure there is a test covering this case.","path":"packages/codemod/src/react/v2.0.0/import-react-icons.js","line":32},{"body":"Changing both `dark` and `light` color modes to \'gray:50\' removes the distinction between the two modes. Is this intentional? If it\'s not actually necessary for the \'light\' mode to be the same as \'dark\', consider keeping \'gray:30\' for \'light\'.","path":"packages/react-docs/pages/components/resize-handle/resizable.js","line":15},{"body":"Consider adding a transition for the opacity change to make the animation smoother when the user hovers over the resizable handle.","path":"packages/react-docs/pages/components/resize-handle/resizable.js","line":78},{"body":"The change from `minSize: 40` to `minSize: 80` could affect the layout and appearance of the table columns. Ensure this adjustment has been reflected in the relevant tests to validate any impact on the table rendering.","path":"packages/react-docs/pages/components/table/backup/column-ordering-using-react-dnd.js","line":127},{"body":"Verify if changing the `minSize` from `40` to `80` does not affect any existing layout or visual structure adversely. Ensure that all relevant tests adequately cover this change in the minimum size of columns.","path":"packages/react-docs/pages/components/table/backup/row-ordering-dnd.js","line":176},{"body":"Adding `minSize: 48` seems redundant since `size` is already set to 48. Consider whether both properties are necessary.","path":"packages/react-docs/pages/components/table/backup/row-ordering-using-react-movable.js","line":81},{"body":"`minSize` change from 48 to 80 may affect the layout and usability of the table. Please ensure this change aligns with the overall design and user experience requirements.","path":"packages/react-docs/pages/components/table/backup/row-ordering-using-react-movable.js","line":135},{"body":"Consider providing a rationale for changing `minSize` from 40 to 80 in the associated test cases to ensure the change is expected and correctly handled.","path":"packages/react-docs/pages/components/table/basic.js","line":56},{"body":"This line changes `minSize` from 40px to 80px in the comment. Ensure that this change is intentional and that it reflects the actual behavior or constraint in the code handling `minSize`.","path":"packages/react-docs/pages/components/table/column-auto-sizing.js","line":125},{"body":"The inline comment mentions the minimum size as \'80px\'. Ensure that the updated minimum size value is consistent across the codebase and that any related documentation is also updated to reflect this change.","path":"packages/react-docs/pages/components/table/column-auto-sizing.js","line":151},{"body":"The `minSize` property has been changed from 40 to 80. Ensure this is necessary for the test coverage improvement and doesn\'t inadvertently alter the intended layout or functionality.","path":"packages/react-docs/pages/components/table/column-ordering-dnd.js","line":201},{"body":"Verify if the updated minimum size value is consistent with the rest of the code and requirements. Adjust any related values or logic that depend on this minimum size.","path":"packages/react-docs/pages/components/table/column-ordering-dnd.js","line":264},{"body":"The change from `minSize: 40` to `minSize: 80` will affect the minimum column width. Ensure that the updated minimum size fits within the design requirements, as this may cause layout issues if not properly accounted for.","path":"packages/react-docs/pages/components/table/column-resizing.js","line":92},{"body":"Changing the `variant` from `\'outline\'` to `\'default\'` will likely change the visual presentation of the component. Ensure that new tests cover the default variant to validate this change.","path":"packages/react-docs/pages/components/table/column-resizing.js","line":103},{"body":"Review the choice of \'gray:50\' for both \'dark\' and \'light\' modes. Consider if different shades might better match the overall theme for each mode.","path":"packages/react-docs/pages/components/table/column-resizing.js","line":150},{"body":"The `height` value \'9x\' seems unusual and might be a typo. Please confirm if this is the intended value or if it should be a specific height in \'px\', \'em\', etc.","path":"packages/react-docs/pages/components/table/column-resizing.js","line":166},{"body":"The change from `minSize: 40` to `minSize: 80` has doubled the minimum size of the columns. Please ensure this change is intentional and that it aligns with the design specifications and requirements of the table layout. Otherwise, this might impact the table\'s appearance and usability.","path":"packages/react-docs/pages/components/table/column-visibility-and-order/index.js","line":123},{"body":"The change in the comment from `40px` to `80px` should be aligned with the actual code or configuration that sets `minSize`. Please verify if the `minSize` value in the code has been updated accordingly to `80px`.","path":"packages/react-docs/pages/components/table/column-visibility-and-order/index.js","line":178},{"body":"Ensure that changing the example minimum size in this comment from 40px to 80px aligns with actual intended default values used throughout the codebase. Verify if there are other parts of the code where the minSize is referenced and update those if necessary.","path":"packages/react-docs/pages/components/table/column-visibility-and-order/index.js","line":204},{"body":"Increasing the `minSize` from 40 to 80 could impact the layout and usability of the table. Ensure that this change aligns with the design requirements, as it could potentially reduce flexibility in column resizing.","path":"packages/react-docs/pages/components/table/column-visibility.js","line":64},{"body":"Changing `minSize` from 40 to 80 might have a significant impact on the layout and usability of the table. Please ensure this change is well-informed and tested for different screen sizes and data scenarios.","path":"packages/react-docs/pages/components/table/pagination.js","line":85},{"body":"The addition of `minSize` property seems logical, but please verify if this change aligns with the prop types and default values defined for this property.","path":"packages/react-docs/pages/components/table/row-expanding.js","line":142},{"body":"Increasing `minSize` from 40 to 80 might affect the layout significantly. Ensure that this change aligns with the design requirements and does not cause any unintended side-effects in the UI.","path":"packages/react-docs/pages'... 11191 more characters, ``` </details></td></tr></table> <hr> <details><summary><strong>✨ CI feedback usage guide:</strong></summary><hr> The CI feedback tool (`/checks)` automatically triggers when a PR has a failed check. The tool analyzes the failed checks and provides several feedbacks: - Failed stage - Failed test name - Failure summary - Relevant error logs In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: ``` /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}" ``` where `{repo_name}` is the name of the repository, `{run_number}` is the run number of the failed check, and `{job_number}` is the job number of the failed check. #### Configuration options - `enable_auto_checks_feedback` - if set to true, the tool will automatically provide feedback when a check is failed. Default is true. - `excluded_checks_list` - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list. - `enable_help_text` - if set to true, the tool will provide a help message with the feedback. Default is true. - `persistent_comment` - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true. - `final_update_message` - if `persistent_comment` is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true. See more information about the `checks` tool in the [docs](https://pr-agent-docs.codium.ai/tools/ci_feedback/). </details> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cheton"><img src="https://avatars.githubusercontent.com/u/447801?v=4" />cheton</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>/improve</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>PR Code Suggestions ✨</h2> <table><thead><tr><td>Category</td><td align=left>Suggestion </td><td align=center>Score</td></tr><tbody><tr><td rowspan=1><strong>Accessibility</strong></td> <td> <details><summary>Add an <code>aria-labelledby</code> attribute to improve accessibility</summary> ___ Consider adding an <code>aria-labelledby</code> attribute to the <code>AccordionContent</code> component to improve <br>accessibility by associating the region with a header. [packages/react/src/accordion/AccordionContent.js [32]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-4c791ff03de79e3b5b99001930551c285d93dee24755879f0fdcbc30448a2c31R32-R32) ```diff -role="region" +role="region" aria-labelledby={context?.accordionHeaderId} ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 8</summary> Why: The suggestion to add `aria-labelledby` is crucial for accessibility, linking the content region to its header, which is a significant improvement. </details></details></td><td align=center>8 </td></tr><tr><td rowspan=3><strong>Test</strong></td> <td> <details><summary>Add a test case to verify the presence of the <code>role="region"</code> attribute</summary> ___ Add a test case to verify that the <code>AccordionContent</code> component has the <code>role="region"</code> <br>attribute to ensure that accessibility features are correctly implemented. [packages/react/src/accordion/__tests__/Accordion.test.js [16]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR16-R16) ```diff it('should render correctly', async () => { + const region = screen.getByRole('region'); + expect(region).toBeInTheDocument(); ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 7</summary> Why: Adding a test to check for the `role="region"` attribute ensures that accessibility features are implemented correctly. It's a good practice to validate such attributes in unit tests. </details></details></td><td align=center>7 </td></tr><tr><td> <details><summary>Add assertions to check the <code>aria-expanded</code> attribute for better state validation</summary> ___ In the test case for toggling visibility of accordion items, add assertions to check the <br><code>aria-expanded</code> attribute of the headers to ensure they reflect the correct state. [packages/react/src/accordion/__tests__/Accordion.test.js [81]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR81-R81) ```diff expect(body1).toBeVisible(); +expect(header1).toHaveAttribute('aria-expanded', 'true'); ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 7</summary> Why: Verifying the `aria-expanded` attribute in tests is important for accessibility state management, ensuring that the UI reflects the correct state of components. </details></details></td><td align=center>7 </td></tr><tr><td> <details><summary>Add a test case to verify the <code>aria-label</code> attribute for accessibility</summary> ___ Add a test case to verify that the <code>Textarea</code> component correctly applies the <code>aria-label</code> <br>attribute when provided, to ensure accessibility labels are handled properly. [packages/react/src/textarea/__tests__/Textarea.test.js [9]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR9-R9) ```diff +it('should render correctly with aria-label', async () => { + render(<Textarea data-testid="textarea" aria-label="Sample Textarea" />); + const textarea = screen.getByTestId('textarea'); + expect(textarea).toHaveAttribute('aria-label', 'Sample Textarea'); +}); + it('should render correctly', async () => { ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=3 --> <details><summary>Suggestion importance[1-10]: 7</summary> Why: Testing for the presence of `aria-label` is essential for accessibility, ensuring that form elements are properly labeled for screen reader users. </details></details></td><td align=center>7 </td></tr></tr></tbody></table> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cheton"><img src="https://avatars.githubusercontent.com/u/447801?v=4" />cheton</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>/review --pr_reviewer.inline_code_comments=true --pr_reviewer.require_score_review=true</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cheton"><img src="https://avatars.githubusercontent.com/u/447801?v=4" />cheton</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>/improve</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p><strong><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870#issuecomment-2167596704">Persistent review</a></strong> updated to latest commit <a href="https://github.com/trendmicro-frontend/tonic-ui/commit/aae0fea4798ff6f1088fe2fda342a5e792b0baa3">https://github.com/trendmicro-frontend/tonic-ui/commit/aae0fea4798ff6f1088fe2fda342a5e792b0baa3</a></p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>PR Code Suggestions ✨</h2> <table><thead><tr><td>Category</td><td align=left>Suggestion </td><td align=center>Score</td></tr><tbody><tr><td rowspan=1><strong>Best practice</strong></td> <td> <details><summary>Conditionally add the <code>role="region"</code> attribute based on the expanded state</summary> ___ Ensure that the <code>role="region"</code> attribute is conditionally added only when <br><code>context?.isExpanded</code> is true. This will prevent unnecessary roles being added when the <br>accordion content is not expanded. [packages/react/src/accordion/AccordionContent.js [32]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-4c791ff03de79e3b5b99001930551c285d93dee24755879f0fdcbc30448a2c31R32-R32) ```diff -role="region" +role={context?.isExpanded ? "region" : undefined} ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 8</summary> Why: This suggestion improves accessibility by ensuring that the `role="region"` is only added when the accordion is expanded, which is a significant improvement in the context of accessibility best practices. </details></details></td><td align=center>8 </td></tr><tr><td rowspan=3><strong>Test coverage</strong></td> <td> <details><summary>Add a test case to verify the conditional application of the <code>role="region"</code> attribute</summary> ___ Add a test case to verify that the <code>role="region"</code> attribute is correctly applied to the <br>expanded accordion content and not applied to collapsed content. [packages/react/src/accordion/__tests__/Accordion.test.js [15-54]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR15-R54) ```diff it('should render correctly', async () => { const renderOptions = { useCSSVariables: true, }; const { container } = render(( <Accordion rowGap={1}> <AccordionItem defaultIsExpanded> <AccordionHeader> <Text> Accordion 1 </Text> </AccordionHeader> <AccordionBody> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> <AccordionItem> <AccordionHeader> <Text> Accordion 2 </Text> </AccordionHeader> <AccordionBody> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> </Accordion> ), renderOptions); expect(container).toMatchSnapshot(); await testA11y(container, { axeOptions: { rules: { label: { enabled: false }, }, }, }); + + const body1 = screen.getByText('Accordion 1').closest('div').nextSibling; + const body2 = screen.getByText('Accordion 2').closest('div').nextSibling; + expect(body1).toHaveAttribute('role', 'region'); + expect(body2).not.toHaveAttribute('role'); }); ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 7</summary> Why: The suggestion correctly identifies the need for additional test coverage to ensure the conditional application of `role="region"` works as expected. This is crucial for maintaining the quality and reliability of the component. </details></details></td><td align=center>7 </td></tr><tr><td> <details><summary>Add a test case to verify that focusable elements within <code>VisuallyHidden</code> are accessible</summary> ___ Add a test case to verify that the <code>VisuallyHidden</code> component correctly handles focusable <br>elements, ensuring they are still accessible to keyboard navigation. [packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js [17-36]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-0e247f86dd40408f95235c167d00efc8abc097e6704a3d6f2ff991246f1202b7R17-R36) ```diff it('should render the content but visually hidden', () => { const { getByText } = render( <VisuallyHidden>Hidden Content</VisuallyHidden> ); const content = getByText('Hidden Content'); // Check that the content is in the document expect(content).toBeInTheDocument(); // Check that the content is visually hidden const styles = window.getComputedStyle(content); expect(styles.position).toBe('absolute'); expect(styles.width).toBe('1px'); expect(styles.height).toBe('1px'); expect(styles.padding).toBe('0px'); expect(styles.border).toBe('0px'); expect(styles.overflow).toBe('hidden'); expect(styles.clipPath).toBe('inset(50%)'); expect(styles.whiteSpace).toBe('nowrap'); }); +it('should render focusable elements correctly', () => { + const { getByText } = render( + <VisuallyHidden> + <button>Hidden Button</button> + </VisuallyHidden> + ); + const button = getByText('Hidden Button'); + + // Check that the button is in the document + expect(button).toBeInTheDocument(); + + // Check that the button is focusable + button.focus(); + expect(document.activeElement).toBe(button); +}); + ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 7</summary> Why: This suggestion is valuable as it addresses the accessibility of focusable elements within visually hidden components, ensuring they remain operable, which is crucial for accessibility compliance. </details></details></td><td align=center>7 </td></tr><tr><td> <details><summary>Add a test case to verify the <code>placeholder</code> attribute in the <code>Textarea</code> component</summary> ___ Add a test case to verify that the <code>Textarea</code> component correctly handles the <code>placeholder</code> <br>attribute. [packages/react/src/textarea/__tests__/Textarea.test.js [82-95]](https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR82-R95) ```diff it('should render correctly with required attributes', async () => { const { container } = render( <Textarea data-testid="textarea" required /> ); await testA11y(container, { axeOptions: { rules: { label: { enabled: false }, }, }, }); const textarea = screen.getByTestId('textarea'); expect(textarea).toHaveAttribute('aria-required', 'true'); }); +it('should render correctly with placeholder attribute', async () => { + render( + <Textarea data-testid="textarea" placeholder="Enter text here" /> + ); + const textarea = screen.getByTestId('textarea'); + expect(textarea).toHaveAttribute('placeholder', 'Enter text here'); +}); + ``` - [ ] Apply this suggestion <!-- /improve --apply_suggestion=3 --> <details><summary>Suggestion importance[1-10]: 6</summary> Why: Adding a test for the `placeholder` attribute is a good practice to ensure it functions correctly, although it's a relatively minor enhancement in the context of the overall component testing. </details></details></td><td align=center>6 </td></tr></tr></tbody></table> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cheton"><img src="https://avatars.githubusercontent.com/u/447801?v=4" />cheton</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>/analyze</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>PR Analysis 🔬</h2> <ul> <li>This screen contains a list of code components that were changed in this PR.</li> <li>You can initiate specific actions for each component, by checking the relevant boxes.</li> <li>After you check a box, the action will be performed automatically by PR-Agent.</li> <li>Results will appear as a comment on the PR, typically after 30-60 seconds.</li> </ul> <table><thead><tr><th>file</th><th align="center">Changed components</th></tr></thead><tbody><tr><td><strong>AccordionContent.js</strong></td><td><table><tr> <td> - [ ] Test <!-- /test arrow_function -__-pr_test.file=packages/react/src/accordion/AccordionContent.js --> - [ ] Docs <!-- /add_docs arrow_function -__-pr_add_docs.file=packages/react/src/accordion/AccordionContent.js --> - [ ] Improve <!-- /improve_component arrow_function -__-pr_improve_component.file=packages/react/src/accordion/AccordionContent.js --> - [ ] Similar <!-- /find_similar_component arrow_function -__-pr_similar_code.file=packages/react/src/accordion/AccordionContent.js --> </td> <td><dd><sup> </sup></dd><strong>arrow_function</strong> <dd><sup>(function)</sup></dd><td align="left"><dd><sup> </sup></dd><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-4c791ff03de79e3b5b99001930551c285d93dee24755879f0fdcbc30448a2c31R32">+2/-1</a> <dd><sup> </sup></dd> </td> </tr></table></td></tr><tr><td><strong>Accordion.test.js</strong></td><td><table><tr> <td> - [x] Test <!-- /test arrow_function -__-pr_test.file=packages/react/src/accordion/__tests__/Accordion.test.js --> - [x] Docs <!-- /add_docs arrow_function -__-pr_add_docs.file=packages/react/src/accordion/__tests__/Accordion.test.js --> - [x] Improve <!-- /improve_component arrow_function -__-pr_improve_component.file=packages/react/src/accordion/__tests__/Accordion.test.js --> - [x] Similar <!-- /find_similar_component arrow_function -__-pr_similar_code.file=packages/react/src/accordion/__tests__/Accordion.test.js --> </td> <td><dd><sup> </sup></dd><strong>arrow_function</strong> <dd><sup>(function)</sup></dd><td align="left"><dd><sup> </sup></dd><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR15">+175/-0</a> <dd><sup> </sup></dd> </td> </tr></table></td></tr><tr><td><strong>Textarea.test.js</strong></td><td><table><tr> <td> - [ ] Test <!-- /test arrow_function -__-pr_test.file=packages/react/src/textarea/__tests__/Textarea.test.js --> - [ ] Docs <!-- /add_docs arrow_function -__-pr_add_docs.file=packages/react/src/textarea/__tests__/Textarea.test.js --> - [ ] Improve <!-- /improve_component arrow_function -__-pr_improve_component.file=packages/react/src/textarea/__tests__/Textarea.test.js --> - [ ] Similar <!-- /find_similar_component arrow_function -__-pr_similar_code.file=packages/react/src/textarea/__tests__/Textarea.test.js --> </td> <td><dd><sup> </sup></dd><strong>arrow_function</strong> <dd><sup>(function)</sup></dd><td align="left"><dd><sup> </sup></dd><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-86a773dab4d14773dfe69929462b27070d0bb91aefb0aa0dfa8f7226b91fdf2dR8">+128/-0</a> <dd><sup> </sup></dd> </td> </tr></table></td></tr><tr><td><strong>VisuallyHidden.test.js</strong></td><td><table><tr> <td> - [ ] Test <!-- /test arrow_function -__-pr_test.file=packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js --> - [ ] Docs <!-- /add_docs arrow_function -__-pr_add_docs.file=packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js --> - [ ] Improve <!-- /improve_component arrow_function -__-pr_improve_component.file=packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js --> - [ ] Similar <!-- /find_similar_component arrow_function -__-pr_similar_code.file=packages/react/src/visually-hidden/__tests__/VisuallyHidden.test.js --> </td> <td><dd><sup> </sup></dd><strong>arrow_function</strong> <dd><sup>(function)</sup></dd><td align="left"><dd><sup> </sup></dd><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-0e247f86dd40408f95235c167d00efc8abc097e6704a3d6f2ff991246f1202b7R6">+32/-0</a> <dd><sup> </sup></dd> </td> </tr></table></td></tr></tbody></table> <hr> <details> <summary><strong>💡 Usage guide:</strong></summary><hr> Using static code analysis capabilities, the `analyze` tool scans the PR code changes and find the code components (methods, functions, classes) that changed in the PR. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR: ``` /analyze ``` Language that are currently supported: Python, Java, C++, JavaScript, TypeScript, C#. See more information about the tool in the [docs](https://pr-agent-docs.codium.ai/tools/analyze/). </details> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>Generated tests for 'arrow_function' ✏️️</h2> <ul> <strong>arrow_function</strong> (function) <sup><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR15"> [+175/-0]</a></sup><br><br> <strong>Component signature:</strong> ```javascript describe('Accordion', () => { ``` <br><strong>Tests for code changes in `arrow_function` function:</strong> <table><tr><td><details> <summary>[happy path]<dd>The accordion should render correctly with the specified structure and accessibility attributes</dd> </summary> <hr> <strong>test_code:</strong> ```javascript import { render, screen } from '@testing-library/react'; import { Accordion, AccordionItem, AccordionHeader, AccordionBody, Box, Text } from '@tonic-ui/react/src'; import { testA11y } from '@tonic-ui/react/test-utils/accessibility'; import React from 'react'; it('should render accordion correctly with accessibility attributes', async () => { const renderOptions = { useCSSVariables: true, }; const { container } = render(( <Accordion rowGap={1}> <AccordionItem defaultIsExpanded> <AccordionHeader> <Text> Accordion 1 </Text> </AccordionHeader> <AccordionBody> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> <AccordionItem> <AccordionHeader> <Text> Accordion 2 </Text> </AccordionHeader> <AccordionBody> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> </Accordion> ), renderOptions); expect(container).toMatchSnapshot(); await testA11y(container, { axeOptions: { rules: { label: { enabled: false }, }, }, }); }); ``` </details> </td></tr> <tr><td><details> <summary>[happy path]<dd>The accordion items should toggle visibility independently when clicked</dd> </summary> <hr> <strong>test_code:</strong> ```javascript import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Accordion, AccordionItem, AccordionHeader, AccordionBody, Box, Text } from '@tonic-ui/react/src'; import React from 'react'; it('should toggle visibility of accordion items independently', async () => { const user = userEvent.setup(); const TestComponent = () => { const [expandedItem, setExpandedItem] = React.useState('item1'); const handleToggle = item => ({ isExpanded }) => { setExpandedItem(isExpanded ? item : null); }; return ( <Accordion rowGap={1}> <AccordionItem isExpanded={expandedItem === 'item1'} onToggle={handleToggle('item1')} > <AccordionHeader data-testid="header-1"> <Text> Accordion 1 </Text> </AccordionHeader> <AccordionBody data-testid="body-1"> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> <AccordionItem isExpanded={expandedItem === 'item2'} onToggle={handleToggle('item2')} > <AccordionHeader data-testid="header-2"> <Text> Accordion 2 </Text> </AccordionHeader> <AccordionBody data-testid="body-2"> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> </Accordion> ); }; render(<TestComponent />); const header1 = screen.getByTestId('header-1'); const body1 = screen.getByTestId('body-1'); const header2 = screen.getByTestId('header-2'); const body2 = screen.getByTestId('body-2'); expect(header1).toHaveAttribute('aria-expanded', 'true'); expect(header2).not.toHaveAttribute('aria-expanded'); expect(body1).not.toHaveAttribute('aria-hidden'); expect(body2).toHaveAttribute('aria-hidden', 'true'); expect(body1).toBeVisible(); expect(body2).not.toBeVisible(); // Toggle the second accordion item await user.click(header2); expect(header1).not.toHaveAttribute('aria-expanded'); expect(header2).toHaveAttribute('aria-expanded', 'true'); expect(body1).toHaveAttribute('aria-hidden', 'true'); expect(body2).not.toHaveAttribute('aria-hidden'); expect(body1).not.toBeVisible(); expect(body2).toBeVisible(); // Toggle the first accordion item await user.click(header1); expect(header1).toHaveAttribute('aria-expanded', 'true'); expect(header2).not.toHaveAttribute('aria-expanded'); expect(body1).not.toHaveAttribute('aria-hidden'); expect(body2).toHaveAttribute('aria-hidden', 'true'); expect(body1).toBeVisible(); expect(body2).not.toBeVisible(); }); ``` </details> </td></tr> <tr><td><details> <summary>[edge case]<dd>The accordion should render correctly with the disabled attribute</dd> </summary> <hr> <strong>test_code:</strong> ```javascript import { render, screen } from '@testing-library/react'; import { AccordionItem, AccordionHeader, AccordionBody, Box, Text } from '@tonic-ui/react/src'; import React from 'react'; it('should render accordion correctly with disabled attribute', () => { render( <AccordionItem disabled> <AccordionHeader data-testid="accordion-header"> <Text> Disabled Accordion </Text> </AccordionHeader> <AccordionBody> <Box px="4x" py="3x" /> </AccordionBody> </AccordionItem> ); const accordionHeader = screen.getByTestId('accordion-header'); expect(accordionHeader).toBeDisabled(); }); ``` </details> </td></tr> </table> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>Generated docstring for 'arrow_function' 📚</h2> <tbody> <ul> <strong>arrow_function</strong> (function) <sup><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR15"> [+175/-0]</a></sup> <br><br> <strong>Component signature:</strong><br><br> ```javascript const arrow_function = () => { ``` <strong>Docstring:</strong><br> ```javascript / * Test suite for the Accordion component. * * @function * @name arrow_function * @description This function contains multiple test cases to ensure the Accordion component renders correctly, toggles visibility of items when clicked, handles multiple items independently, and respects the disabled attribute. / ``` <br> </ul> </tbody> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h2>Generated code suggestions for 'arrow_function'</h2> <ul> <strong>arrow_function</strong> (function) <sup><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files#diff-a7403b6784186b493fb176a03715e9b716abd2a531d6e2df306a9014f5866a1cR15"> [+175/-0]</a></sup><br><br> <strong>Component signature:</strong> ```javascript describe('Accordion', () => { ``` <br><strong>Suggestions and improvements for code changes in `arrow_function` function:</strong> <table><thead><tr><td>Category</td><td align=left>Suggestions </td></tr></thead><tbody><tr><td rowspan=2><strong>Enhancement</strong></td> <td> <details><summary>Add a test case to verify that clicking an already expanded accordion item collapses it.</summary> ___ Consider adding a test case to verify that clicking an already expanded accordion item <br>collapses it. This will ensure that the toggle functionality works both ways.* [packages/react/src/accordion/__tests__/Accordion.test.js ](https://github.com/trendmicro-frontend/tonic-ui/blob/aae0fea4798ff6f1088fe2fda342a5e792b0baa3/packages/react/src/accordion/__tests__/Accordion.test.js/#L15-L189) ```diff // Toggle the first accordion item await user.click(header1); expect(header1).toHaveAttribute('aria-expanded', 'true'); expect(body1).not.toHaveAttribute('aria-hidden'); expect(body1).toBeVisible(); +// Collapse the first accordion item +await user.click(header1); +expect(header1).not.toHaveAttribute('aria-expanded'); +expect(body1).toHaveAttribute('aria-hidden', 'true'); +expect(body1).not.toBeVisible(); + ``` <tr><td> <details><summary>Add a test case to verify that accordion items are not expanded by default.</summary> ___ Add a test case to verify that the accordion items are not expanded by default when <br><code>defaultIsExpanded</code> is not set. [packages/react/src/accordion/__tests__/Accordion.test.js ](https://github.com/trendmicro-frontend/tonic-ui/blob/aae0fea4798ff6f1088fe2fda342a5e792b0baa3/packages/react/src/accordion/__tests__/Accordion.test.js/#L15-L189) ```diff it('should render correctly', async () => { +it('should not expand accordion items by default', async () => { + render( + <Accordion> + <AccordionItem> + <AccordionHeader data-testid="header-1">Section 1</AccordionHeader> + <AccordionBody data-testid="body-1">Content 1</AccordionBody> + </AccordionItem> + </Accordion> + ); + const header1 = screen.getByTestId('header-1'); + const body1 = screen.getByTestId('body-1'); + + expect(header1).not.toHaveAttribute('aria-expanded'); + expect(body1).toHaveAttribute('aria-hidden', 'true'); + expect(body1).not.toBeVisible(); +}); + ``` </details></td></tr><tr><td rowspan=1><strong>Best practice</strong></td> <td> <details><summary>Use <code>screen.findByTestId</code> for elements that appear after an asynchronous action.</summary> ___ Use <code>screen.findByTestId</code> instead of <code>screen.getByTestId</code> for elements that appear after an <br>asynchronous action to ensure the test waits for the elements to be available. [packages/react/src/accordion/__tests__/Accordion.test.js ](https://github.com/trendmicro-frontend/tonic-ui/blob/aae0fea4798ff6f1088fe2fda342a5e792b0baa3/packages/react/src/accordion/__tests__/Accordion.test.js/#L15-L189) ```diff -const header1 = screen.getByTestId('header-1'); -const body1 = screen.getByTestId('body-1'); +const header1 = await screen.findByTestId('header-1'); +const body1 = await screen.findByTestId('body-1'); ``` </details></td></tr><tr><td rowspan=1><strong>Maintainability</strong></td> <td> <details><summary>Remove the duplicate assertion for visibility of the second accordion body.</summary> ___ Remove the duplicate <code>expect(body2).toBeVisible();</code> assertion in the test case for handling <br>multiple accordion items independently. [packages/react/src/accordion/__tests__/Accordion.test.js ](https://github.com/trendmicro-frontend/tonic-ui/blob/aae0fea4798ff6f1088fe2fda342a5e792b0baa3/packages/react/src/accordion/__tests__/Accordion.test.js/#L15-L189) ```diff -expect(body2).toBeVisible(); expect(body2).toBeVisible(); ``` </details></td></tr></tr></tbody></table> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cheton"><img src="https://avatars.githubusercontent.com/u/447801?v=4" />cheton</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>/ask Revise the PR title</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <h3><strong>Ask</strong>❓</h3> <p>Revise the PR title</p> <h3><strong>Answer:</strong></h3> <p>The current PR title is: 'test(react): improve test coverage for <code>Accordion</code>, <code>Textarea</code>, and <code>VisuallyHidden</code> components'.</p> <p>Here is a revised version of the title that is more descriptive and aligns with conventional commit messages:</p> <p><strong>Revised Title:</strong></p> <pre><code>test(react): enhance test coverage and accessibility for `Accordion`, `Textarea`, and `VisuallyHidden` components</code></pre> <p>This revised title better reflects the scope of the changes, including the focus on both test coverage and accessibility improvements.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cheton"><img src="https://avatars.githubusercontent.com/u/447801?v=4" />cheton</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>/describe --pr_description.extra_instructions=" For the title, use the format [type]: [summary] " --pr_description.generate_ai_title=true</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/codiumai-pr-agent-pro[bot]"><img src="https://avatars.githubusercontent.com/in/484649?v=4" />codiumai-pr-agent-pro[bot]</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p><strong><a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870">PR Description</a></strong> updated to latest commit (<a href="https://github.com/trendmicro-frontend/tonic-ui/commit/aae0fea4798ff6f1088fe2fda342a5e792b0baa3">https://github.com/trendmicro-frontend/tonic-ui/commit/aae0fea4798ff6f1088fe2fda342a5e792b0baa3</a>)</p> <ul> <li>[ ] Copy walkthrough table to <a href="https://github.com/trendmicro-frontend/tonic-ui/pull/870/files">"Files Changed"</a> Tab <!-- /describe -_****_-copy_walkthrough_to_diffview_tab --></li> </ul> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>

trendmicro-frontend / tonic-ui

test(react): improve test coverage for `Accordion`, `Textarea`, and `VisuallyHidden` components #870

PR Type

Description

Changes walkthrough 📝

Review or Edit in CodeSandbox

Tonic UI Demo

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/trendmicro-frontend/tonic-ui/commit/aae0fea4798ff6f1088fe2fda342a5e792b0baa3)

PR Documentation 📚

PR Code Suggestions ✨

Textarea.test.js

VisuallyHidden.test.js

Codecov Report

PR Code Suggestions ✨