chore(Form/FormGroup): Include OUIAProps for Form and FormGroup#12426
Open
mshriver wants to merge 2 commits into
Open
chore(Form/FormGroup): Include OUIAProps for Form and FormGroup#12426mshriver wants to merge 2 commits into
mshriver wants to merge 2 commits into
Conversation
WalkthroughThis PR adds OUIA support to Form and FormGroup: both components accept optional ChangesOUIA Support for Form Components
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
|
Preview: https://pf-react-pr-12426.surge.sh A11y report: https://pf-react-pr-12426-a11y.surge.sh |
There was a problem hiding this comment.
Pull request overview
Adds Open UI Automation (OUIA) attribute support to the Form and FormGroup components in react-core to improve automated testability, and updates related documentation/tests to reflect the new attributes.
Changes:
- Add
OUIAPropssupport anduseOUIAPropsintegration toFormandFormGroup. - Update OUIA documentation list to include
FormandFormGroup. - Update relevant Jest snapshots to include the new
data-ouia-*attributes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/react-core/src/helpers/OUIA/OUIA.md | Adds Form and FormGroup to the documented list of OUIA-compliant components. |
| packages/react-core/src/components/Form/Form.tsx | Implements OUIA props on the <form> root element. |
| packages/react-core/src/components/Form/FormGroup.tsx | Implements OUIA props on the FormGroup root <div>. |
| packages/react-core/src/components/LoginPage/tests/snapshots/LoginForm.test.tsx.snap | Updates snapshots to include OUIA attributes emitted by Form/FormGroup (and resulting generated IDs). |
| packages/react-core/src/components/Form/tests/snapshots/Form.test.tsx.snap | Updates Form snapshots for new OUIA attributes. |
| packages/react-core/src/components/Form/tests/snapshots/FormGroup.test.tsx.snap | Updates FormGroup snapshots for new OUIA attributes (and nested Form usage in some cases). |
| packages/react-core/src/components/Form/tests/snapshots/ActionGroup.test.tsx.snap | Updates snapshot due to Form now emitting OUIA attributes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+43
to
+47
| [cssMaxWidth.name]: maxWidth, | ||
| ...props.style | ||
| } as React.CSSProperties | ||
| })} | ||
| {...props} |
Author
There was a problem hiding this comment.
This is true for the main branch and I'd rather not include this change to the props processing behavior in this PR
mshriver
changed the title
mshriver
changed the title
Co-authored-by: Claude <noreply@anthropic.com>
Includes Form and FormGroup ActionGroup and LoginForm too, as they use Forms
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-core/src/components/Form/Form.tsx (1)
37-37: ⚡ Quick winConsider adding OUIA test coverage for
Form.New OUIA-enabled components in this repo typically ship with tests asserting the
data-ouia-component-type/data-ouia-component-id/data-ouia-safeattributes (e.g. honoring a passedouiaIdand theouiaSafedefault). Adding one would lock in the contract.Want me to draft a test for the OUIA attributes on
Form?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-core/src/components/Form/Form.tsx` at line 37, Add a unit test for the Form component to assert it renders OUIA attributes from useOUIAProps: render/mount Form (import Form from Form.tsx) with a specific ouiaId prop and without passing ouiaSafe, then assert the rendered root element has data-ouia-component-type equal to Form.displayName (or "Form"), data-ouia-component-id equal to the passed ouiaId, and data-ouia-safe equals the default value; also add a second assertion toggling ouiaSafe to false to confirm it changes. Use the existing test utilities (e.g., render from `@testing-library/react` or the repo's test helpers) and reference Form and useOUIAProps in the test description so the contract is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/react-core/src/components/Form/Form.tsx`:
- Line 37: Add a unit test for the Form component to assert it renders OUIA
attributes from useOUIAProps: render/mount Form (import Form from Form.tsx) with
a specific ouiaId prop and without passing ouiaSafe, then assert the rendered
root element has data-ouia-component-type equal to Form.displayName (or "Form"),
data-ouia-component-id equal to the passed ouiaId, and data-ouia-safe equals the
default value; also add a second assertion toggling ouiaSafe to false to confirm
it changes. Use the existing test utilities (e.g., render from
`@testing-library/react` or the repo's test helpers) and reference Form and
useOUIAProps in the test description so the contract is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0568f90c-3312-4d61-8da0-c82a02a567f8
⛔ Files ignored due to path filters (4)
packages/react-core/src/components/Form/__tests__/__snapshots__/ActionGroup.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Form/__tests__/__snapshots__/Form.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Form/__tests__/__snapshots__/FormGroup.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/LoginPage/__tests__/__snapshots__/LoginForm.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
packages/react-core/src/components/Form/Form.tsxpackages/react-core/src/components/Form/FormGroup.tsxpackages/react-core/src/helpers/OUIA/OUIA.md
✅ Files skipped from review due to trivial changes (1)
- packages/react-core/src/helpers/OUIA/OUIA.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-core/src/components/Form/FormGroup.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What:
Add OUIAProps support to Form and FormGroup, for better test automation support.
First contribution, believe I've followed contributing guidelines.
Opus/Sonnet used for initial commit and mentioned as co-author.
Tear it up! Thanks for review and consideration.
Summary by CodeRabbit
New Features
Documentation