-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[NoQA] Tests for group chat name #46661
base: main
Are you sure you want to change the base?
Conversation
@marcaaron Last one was reverted because of some workflow issues, can you check this? |
@ShridharGoel What caused the workflow issues? |
Some typecheck issues after getting merged (checks had passed in the old PR). #40658 (comment) |
Can you please point to the changes that fixed the workflow issues? |
@ShridharGoel Did you get a chance to check my above comment? (Also please fix ts errors) |
I think the issue was that the signature of |
@ShridharGoel Can you please fix the ts errors |
@ShridharGoel Any updates on this? |
Updated. |
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.
Please refer to tests/ui/UnreadIndicatorsTest.tsx
once again as the logic get simplified
await waitForBatchedUpdatesWithAct(); | ||
} | ||
|
||
function beforeAllSetupUITests(shouldConnectToPusher = false) { |
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.
This is no longer needed. There is setupApp
and setupGlobalFetchMock
that does the same
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.
Is this still needed? We have this mock in jest/setup.ts
@ShridharGoel Any updates? |
@ShridharGoel Did you get a chance to look into this? |
Will check this within 2 days. |
Details
Tests for group chat name.
Fixed Issues
$ #40189
PROPOSAL: #40189 (comment)
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.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 and/or tagged@Expensify/design
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