Display status emojis in the LHN (including the user's own status)#24414
Display status emojis in the LHN (including the user's own status)#24414stitesExpensify merged 34 commits intoExpensify:mainfrom
Conversation
burczu
left a comment
burczu
left a comment
There was a problem hiding this comment.
Reviewed and tested - I think we are good to go!
|
Looking great so far! 2 things that I noticed:
2. In @burczu testing videos, the status is showing up on the `#admins` channel which shouldn't happen. The status should only ever show up on DMs. I was not able to repro this so maybe it isn't happening anymore? But something i wanted to call out in case you have an idea how that could happen @perunt
24414-web.mp4 |
|
I added a check for that second issue. As for the first one, it's unrelated to focus mode like I thought. I can't get it to happen on my end. Would you mind sharing the test account details or that JSON report? |
|
Hmm there is nothing special about the accounts i'm using. @burczu are you able to see the emojis in |
|
@stitesExpensify @perunt Regarding first issue - I think it works correctly: the status icon shows up in LHN only next to items where the current user, that has this status set, is presented:
|
src/libs/ReportUtils.js
Outdated
| !isTaskReport(report) && // | ||
| isDM(report) && | ||
| !isIOUReport(report) && | ||
| report.participantsList.length === 1 |
There was a problem hiding this comment.
I don't think this correctly fixes the issue with showing up status icon next to channels like #admins - in my case there is only one admin, and the icons keeps showing up
There was a problem hiding this comment.
I'm also a bit confused how #admins is passing the other checks here. It should be isDM or !isChatRoom
There was a problem hiding this comment.
I found the issue. The function isChatRoom() doesn't seem to return true even when the report has the field isChatRoom: true 😕
Hmm something must just be weird on my end then. |
|
✋ 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/stitesExpensify in version: 1.3.58-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.58-5 🚀
|
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 1.3.59-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.59-5 🚀
|
| </OfflineWithFeedback> | ||
| )} | ||
| </PressableWithoutFeedback> | ||
| <SignInOrAvatarWithOptionalStatus isCreateMenuOpen={this.props.isCreateMenuOpen} /> |
There was a problem hiding this comment.
@perunt could you elaborate where does this.props.isCreateMenuOpen come from? Or is it always undefined?
There was a problem hiding this comment.
Hmm, tbh I didn't dig where it comes from but now I can see that it was undefined
I see that it was removed (isCreateMenuOpen) without cleaning it inside the component
There was a problem hiding this comment.
Ok thanks for the input.
|
I think that something broke here. I'm no longer seeing the status in LHN at all @perunt |
|
I saw recently the Moment was replaced, it could break it. Let me check |
|
| User.clearDraftCustomStatus(); | ||
| }; | ||
|
|
||
| const navigateBackToSettingsPage = useCallback(() => Navigation.goBack(ROUTES.SETTINGS_PROFILE, false, true), []); |
| <PressableWithoutFeedback | ||
| accessibilityLabel={translate('sidebarScreen.buttonMySettings')} | ||
| accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON} | ||
| onPress={Session.signOutAndRedirectToSignIn} |
There was a problem hiding this comment.
This caused #34764 since signOutAndRedirectToSignIn got the first param as true always. It should have been false instead.
7 participants
Details
This is the first part of a series of PRs to implement the UI for the new custom status feature.
It implements the Status option in the profile settings, and the screen to set a status.
As this feature is incomplete yet and build in iterations it will be hidden behind the
customStatusbeta.Fixed Issues
$ #23132
PROPOSAL:
Tests
Offline tests
QA Steps
Add the customStatus beta to your account or simply let this function return true
Test Steps for Account A:
Test Steps for Account B:
Additional Notes:
Modifying the valid time duration for the status can only be done manually within the code. Refer to the provided link for the specifics.
Verify that no errors appear in the JS console
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
Untitled.2.mov
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android