Bug 1553719 - Trim revision id for compare chooser #5228
Conversation
airimovici
changed the title
airimovici
force-pushed
the
perf-compare-chooser-revision-id-trim
branch
from
1fc3cbd
to
6706d97
Compare
| @@ -19,6 +19,12 @@ export const filterText = { | |||
| inputPlaceholder: 'filter text e.g. linux tp5o', | |||
| }; | |||
|
|
|||
| export const selectorCardText = { | |||
There was a problem hiding this comment.
Since you're moving the messages here to presumably make them reusable, it'd be worth giving the const a more generic name.
Edit: Nm - I realized they're being used for the tests.
| @@ -136,10 +142,12 @@ export default class SelectorCard extends React.Component { | |||
| if (value === '') { | |||
| return this.setState({ validated: false }); | |||
| } | |||
| // trim whitespaces before validating further | |||
There was a problem hiding this comment.
Not really necessary :)
| expect(inputRevision).toBeInTheDocument(); | ||
| expect(inputRevision.value).toBe(valid_hash); | ||
| expect(ref.current.state.invalidRevision).toBeFalsy(); | ||
| expect(ref.current.state.validated).toBeTruthy(); |
There was a problem hiding this comment.
I'm curious why you're using a ref here to check for this text rather than using the react-testing-library queries?
There was a problem hiding this comment.
Thanks for pointing it out. I changed it from asserting the state to checking with the testing-library queries.
|
Thanks for the review, @sarah-clements. I removed the unnecessary comment and changed from checking the state to using the testing-library queries |
|
I think you should be able to merge this (squash and merge keeping relevant commit messages) but if you can't let me know and I'll do it. |
Alex is now on PTO. Thus, I'll perform the squash & merge. |
|
Deployed to production. |
No description provided.