-
Notifications
You must be signed in to change notification settings - Fork 8
OWNER-34 - Make MonthInput's value RTL compatible #1289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OWNER-34 - Make MonthInput's value RTL compatible #1289
Conversation
|
Jira Issue: https://appfolio.atlassian.net/browse/OWNER-34 |
9756571 to
213d280
Compare
213d280 to
0655c03
Compare
0655c03 to
ae6cebd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR makes the MonthInput component's value attribute compatible with React Testing Library by synchronizing the input element's value attribute with its internal state. This change enables DOM-based testing of the component's behavior.
Key Changes:
- Added
setAttribute('value', ...)calls in theonChange,setInputValue, andonBlurmethods to keep the DOM attribute in sync with the input's internal value - Added comprehensive test coverage using React Testing Library to verify the value attribute updates correctly for various user interactions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/components/Input/MonthInput.js | Added setAttribute calls to sync value attribute with internal state; fixed prop name typo |
| src/components/Input/MonthInput.spec.js | Added new RTL compatibility test suite covering initial values, user input, and blur behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ae6cebd to
5341489
Compare
|
Released prerelease version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, I'll use the prerelease of this to QA in tandem with our PR to bundle and report back here later
EDIT: lgtm
5341489 to
9780803
Compare
This change forwards value updates to the input tag's
valueattribute.Prior to this change, the
MonthInputdid not forward it's value to the value attribute of the input tag itself. Because of this, it was impossible to verify behavior of this component via React Testing Library, which relies on DOM-visible state and not the input's internal value.It also corrects the props being passed to the
DropdownMenucomponent to silence an error about invalid attributes being assigned on html tags via Popper.