RangeControl: Convert unit tests to typescript #41445
Merged
Conversation
This file contains 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
aaronrobertshaw
added
[Type] Enhancement
This was referenced May 31, 2022
|
Size Change: 0 B Total Size: 1.25 MB ℹ️ View Unchanged
|
ciampo
approved these changes
May 31, 2022
There was a problem hiding this comment.
Great work! Code changes LGTM, tests still pass, and there's not TS errors 🚀
Optionally, we could look into partially rewriting these tests using @testing-library/user-event (similarly to what done in #40697 and #41270). In case you feel like working on it, I'll leave it up to you if you want to add those changes in this PR, or apply them in a follow-up PR.
The key word "partially" 😄. I had a go at it a few days ago and realized there's no user-event way to manipulate a |
Oh well 😅 We could at least stop using |
|
Thanks for the suggested improvements 👍 I've removed the use of The one catch here was being able to select the icons as they are only spans with no alt, text, or role. I've added a As for leveraging |
I think this is safe, especially since the svg element is But... to be honest I don't think the "should render with icons" test is worth contorting ourselves to avoid a |
|
I'm actually not sure about adding the |
aaronrobertshaw
force-pushed
the
refactor/range-control-to-typescript
branch
from
146713c
to
1f2a73b
Compare
aaronrobertshaw
added a commit
that referenced
this pull request
Jun 6, 2022
aaronrobertshaw
force-pushed
the
refactor/range-control-unit-tests-to-typescript
branch
from
308c826
to
57fb884
Compare
aaronrobertshaw
added a commit
that referenced
this pull request
Jun 7, 2022
aaronrobertshaw
force-pushed
the
refactor/range-control-unit-tests-to-typescript
branch
from
57fb884
to
35005fa
Compare
aaronrobertshaw
force-pushed
the
refactor/range-control-to-typescript
branch
from
6a7bda9
to
a304615
Compare
aaronrobertshaw
added a commit
that referenced
this pull request
Jun 23, 2022
aaronrobertshaw
force-pushed
the
refactor/range-control-unit-tests-to-typescript
branch
from
35005fa
to
54554f1
Compare
aaronrobertshaw
force-pushed
the
refactor/range-control-to-typescript
branch
from
237923b
to
a1fa7d3
Compare
aaronrobertshaw
added a commit
that referenced
this pull request
Jul 3, 2022
See: #40535 (comment) for more information on why we are ignoring this error. TL;DR we are looking to address some conflicts between input control types for their value props.
aaronrobertshaw
force-pushed
the
refactor/range-control-to-typescript
branch
from
a435f81
to
f19616e
Compare
aaronrobertshaw
force-pushed
the
refactor/range-control-unit-tests-to-typescript
branch
from
9471fee
to
1b53957
Compare
aaronrobertshaw
deleted the
refactor/range-control-unit-tests-to-typescript
branch
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.
Depends on:
Related:
What?
Converts the
RangeControlunit tests to typescript.Why?
Completes the process of converting the
RangeControlto TypeScript alongside #40535 & #41444How?
Testing Instructions
npm run test-unit /packages/components/src/range-control/test