-
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
Simplify Global Create menu #25564
Simplify Global Create menu #25564
Conversation
@srikarparsi 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] |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/thienlnam in version: 1.3.71-0 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.3.71-12 🚀
|
We are seeing Room no longer available in "+" menu |
@mvtglobally Room is 2nd tab of Send message. @thienlnam I think we should update TestRail to avoid confusion. |
@situchan Can you share screenshot or video? |
Screen.Recording.2023-09-20.at.11.31.53.PM.mov |
Yeah good call, I created this issue to have the test rails updated https://github.com/Expensify/Expensify/issues/318720 |
<PressableWithFeedback | ||
onPress={() => this.props.onSelectedStatePressed(this.props.option)} | ||
accessibilityRole={CONST.ACCESSIBILITY_ROLE.CHECKBOX} | ||
accessibilityLabel={CONST.ACCESSIBILITY_ROLE.CHECKBOX} | ||
> | ||
<SelectCircle isChecked={this.props.isSelected} /> | ||
</PressableWithFeedback> |
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.
Wrapping with PressableWithFeedback
here caused super minor inconsistency - Cursor Remains as Hand Icon on Checkbox in "Paid By" Section During Split Bill
We should have passed disabled={isDisabled}
.
return; | ||
} | ||
|
||
IOU.setMoneyRequestId(`${iouType.current}${option.reportID}`); | ||
Navigation.navigate(ROUTES.getMoneyRequestConfirmationRoute(iouType.current, option.reportID)); | ||
setHeaderTitle(_.isEmpty(iou.participants) ? translate('tabSelector.manual') : translate('iou.split')); |
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 caused a regression in #28751.
_.isEmpty(iou.participants)
does not necessarily mean that the request is a manual. Manual requests have participants
prop as well. For example, if you select a user and then press back to the participants selector, the title will still be displayed as split
Details
Simplifying Global Create menu is a brand new feature. Refer to this doc for details: https://docs.google.com/document/d/1hNO2XkGnESrrsW238xkc6R4Ndppjm7_bBQC4D7VRuAI/edit#heading=h.ns12dr1f35rx
Fixed Issues
$ #27509
https://github.com/Expensify/Expensify/issues/290194
PROPOSAL: GH_LINK_ISSUE(COMMENT)
Tests
Some of the features are hidden behind beta flags - for example
New Room
functionality. To test them, updatefunction canUseAllBetas(betas) { return true; }
method inPermissions.js
. Remember to also test the feature when beta flags are off.Full suite of manual test scenarios is described here:
https://docs.google.com/document/d/1hNO2XkGnESrrsW238xkc6R4Ndppjm7_bBQC4D7VRuAI/edit#heading=h.j93ukuhgh4on
Offline tests
Full offline support - you should be able to go through the flow as usual. Created chat/group/room/request/split will be grayed out until you become online again. This behaviour is in line with current
main
.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)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)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
Web
Screen.Recording.2023-08-22.at.14.01.15.mov
Mobile Web - Chrome
Screen.Recording.2023-08-22.at.17.54.31.mov
Mobile Web - Safari
Screen.Recording.2023-08-22.at.17.37.40.mov
Desktop
Screen.Recording.2023-08-22.at.17.58.31.mov
iOS
Screen.Recording.2023-08-22.at.17.22.39.mov
Android
Screen.Recording.2023-08-22.at.17.43.38.mov