-
Notifications
You must be signed in to change notification settings - Fork 81
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
Bump Paragon to v22.2.1, fix some bugs that turned up #933
Conversation
Thanks for the pull request, @bradenmacdonald! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #933 +/- ##
==========================================
- Coverage 92.00% 91.93% -0.07%
==========================================
Files 612 571 -41
Lines 10746 10233 -513
Branches 2305 2230 -75
==========================================
- Hits 9887 9408 -479
+ Misses 830 798 -32
+ Partials 29 27 -2 ☔ View full report in Codecov by Sentry. |
@@ -421,7 +421,7 @@ describe('Videos page', () => { | |||
fireEvent.click(screen.getByText(messages.applySortButton.defaultMessage)); | |||
}); | |||
|
|||
const imageFilterChip = screen.getByTestId('icon-after'); | |||
const imageFilterChip = screen.getByRole('button', { name: 'Remove this filter' }); |
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.
The testid
was removed from the "after" button in upstream Paragon. And anyways testid
should be avoided. So I added an explicit alt text using the new iconAfterAlt
attribute, and updated the test to use that.
@KristinAoki Would you be able to review this? |
1f58aac
to
1db067a
Compare
Thanks @KristinAoki! I rebased this just now, and fixed one more snapshot test that needed updating. Could you please merge? I don't have permission. |
@bradenmacdonald 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
This bumps Paragon to v22.2.1, which includes:
<Icon>
,<Chip>
, etc. (feat: working typings for Paragon, better types for <Icon> component paragon#3016)<Chip>
,<SearchField>
,<Pagination>
,<Form.Autosuggest>
(feat!: release updates to Chip, SearchField, Pagination, Form.Autosuggest paragon#2995)Supporting information
I have tried to update everything to resolve the breaking changes. But some things (like the video uploads page) I do not know how to test (they aren't working on my devstack).
Private ref: came from work on FAL-3696