-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 Onyx to 2.0.57 #46297
Bump Onyx to 2.0.57 #46297
Conversation
@sobitneupane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
TS check is failing on |
@hannojg unfortunately, your version – |
Thanks, maybe then lets only bump to 2.057, and then I can check whats wrong, fix it, and then bump my changes separately? |
Yeah, I like the idea of only bumping to 2.0.57 for now. |
@tgolen since there won't be any C+ as it's a bump only – I think, this PR might be ready for merge 🤔 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
@tgolen looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
Test failure was unrelated to the PR. See my previous comment. |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Performance Comparison Report 📊Significant Changes To Duration
Show details
Meaningless Changes To DurationShow entries
Show details
|
@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker. |
I started an internal thread in Slack about this to discuss how we want to handle this. On one hand, it makes sense the change would have slowed some stuff down... but I don't know if this is slowing things down TOO much or not. @paultsimura do you have any initial thoughts, specifically related to opening the chat finder page? |
It looks like this performance regression might not be an issue.
If the performance regression ends up being real, we think it's best to:
For now, I am going to remove the deployblocker label. |
🚀 Cherry-picked to staging by https://github.com/roryabraham in version: 9.0.14-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/roryabraham in version: 9.0.14-6 🚀
|
Details
This is an Onyx bump to
v. 2.0.57
Fixed Issues
$ #37560
PROPOSAL: #37560 (comment)
Tests
Same as QA
Offline tests
Same as QA
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop