Conversation
|
Will add the android and mweb screenshots shortly. |
|
@Pujan92 I was running into an issue with long subtitles wrapping into two lines.
|
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Change makes sense 👍
|
@dannymcclain can you plz help with comment |
|
On second thought maybe given the long emails and potentially long Workspace names it makes more sense two truncate after two lines |
|
Sorry, this got delayed due to the holidays. I’ll update today. |
|
@Pujan92 I have updated the code according to the design. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@Pujan92 pr is ready for review. |
| textStyles={[styles.textAlignCenter]} | ||
| subtitleNumberOfLines={2} | ||
| shouldShowFrom={false} | ||
| textStyles={parentNavigationSubtitleTextStyle} |
There was a problem hiding this comment.
| textStyles={parentNavigationSubtitleTextStyle} | |
| textStyles={parentNavigationSubtitleTextStyle} | |
| descriptionTextStyle={[styles.mutedNormalTextLabel, styles.mb1]} |
We need to set descriptionTextStyle to make it consistent with other description labels.
| shouldShowRightIcon={false} | ||
| interactive={false} | ||
| titleComponent={nameSectionFurtherDetailsContent} | ||
| description={translate('common.from')} |
There was a problem hiding this comment.
| description={translate('common.from')} | |
| description={translate('common.from')} | |
| descriptionTextStyle={[styles.mutedNormalTextLabel, styles.mb1]} |
|
@Nodebrute Also needs to resolve conflicts. |
|
Someone plz trigger the build so @dannymcclain and @dubielzyk-expensify can also verify it |
|
🚧 @dubielzyk-expensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
It looks good for me on an expense and report:
But I'd expect to see the same for a room maybe!?: And then I get this weird left + centered thing on my expense room:
cc @dannymcclain for thoughts on the above |
Same! 👍
I don't think I feel as strongly about this one because there's nothing for you to click on or interact with—though I'm not against separating it out as well. I'd probably do it like this though if we're going to change that:
I didn't come across this, but let's make sure we address it! |
|
Working on it, will update soon |
That makes sense. Happy to proceed with what you're suggesting 👍 |
|
@Pujan92 conflicts resolved. |
Just to highlight, currently we are rendering a common component for non-expense reports(eg. workspace room, invoices) and also alignment is specific to scenarios. To confirm, Do we need to make this format update for workspace room name?
@dannymcclain Can you help to create this expense room, I thought we only have workspace room and there I didn't able to see this issue? |
|
@Expensify/design what do you think? Should we just make this change everywhere? Feels like that would ultimately probably be the right choice, but want to double-check.
I can't repro the issue Jon was having. |
What change are you referring to, Danny? |
|
I think I am following. We should map out how other items would work though... like what do we show for "From" for all possible "chat" types? I guess in most cases we just show the workspace? |
|
Ahh yes I'm down for this 👍 |
|
Works for me! |
|
@Nodebrute Let's make the required change and apply the same for the workspace room |
|
@Pujan92 on it. |
|
Bump @Nodebrute for the update |
|
Sorry, I missed this. I’ll update it today. |
|
@Pujan92 |
|
@Nodebrute we need this specific to workspace room where we have the workspace name. For others, as shown here don't need that change |










Explanation of Change
Fixed Issues
$ #76669
PROPOSAL: #76669 (comment)
Tests
Offline tests
Same as above
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Android: Native
editable version

Android: mWeb Chrome
iOS: Native
non-editable version

editable version

iOS: mWeb Safari
non-editable version

editable version

MacOS: Chrome / Safari
editable version