New welcome message for deleted #admins/#announce rooms #8239
New welcome message for deleted #admins/#announce rooms #8239thesahindia wants to merge 6 commits intoExpensify:mainfrom thesahindia:thesahindia/room-welcome-text
Conversation
…om-welcome-text merge main
|
@rushatgabhane, it's ready for the review now. |
There was a problem hiding this comment.
@thesahindia looking good 🎉 I've requested some code style changes. Let me know if we can do even better.
Also, please add screenshot for iOS - Safari because we have a style change.
Thanks!!
| const policyName = lodashGet(props.policies, [`${ONYXKEYS.COLLECTION.POLICY}${props.report.policyID}`, 'name'], Localize.translateLocal('workspace.common.unavailable')); | ||
|
|
||
| // Show different welcome messages depending on if the room is archived or not and its visiblity. | ||
| function chatRoomWelcomeText() { |
There was a problem hiding this comment.
| function chatRoomWelcomeText() { | |
| function getWelcomeMessage() { |
| // Use the policyName or fallback to default text if it's unavailable. | ||
| const policyName = lodashGet(props.policies, [`${ONYXKEYS.COLLECTION.POLICY}${props.report.policyID}`, 'name'], Localize.translateLocal('workspace.common.unavailable')); | ||
|
|
||
| // Show different welcome messages depending on if the room is archived or not and its visiblity. |
There was a problem hiding this comment.
What do you think about this?
| // Show different welcome messages depending on if the room is archived or not and its visiblity. | |
| /** | |
| * Get welcome message based on room visiblity and archive state. | |
| * @returns .. | |
| */ |
|
|
||
| // Show different welcome messages depending on if the room is archived or not and its visiblity. | ||
| function chatRoomWelcomeText() { | ||
| // eslint-disable-next-line no-nested-ternary |
There was a problem hiding this comment.
@thesahindia let's not do this, it's hard to read.
How about using if else?
| defaultTextProps={{ | ||
| style: styles.textAlignCenter, | ||
| }} | ||
| source={{html: props.translate('reportActionsView.beginningOfChatHistoryPrivate', {reportName: props.report.reportName})}} |
There was a problem hiding this comment.
@thesahindia I believe we can DRY RenderHTML. Because only the source prop is different.
| : `${props.translate('reportActionsView.beginningOfChatHistoryPrivatePartTwo')}`} | ||
| </Text> | ||
| </> | ||
| <RenderHTML |
There was a problem hiding this comment.
Let's move this to getWelcomeMessage()
|
Sorry @rushatgabhane @thesahindia, please pause development here. It has come to my attention that using react-native-render-html in this manner is a pattern that does not have wide consensus. (context) Sorry for potentially leading you down the wrong path here. I'm going to bring this question up in Slack in the next few days and get consensus on the best way to handle this pattern going forward. @thesahindia If you don't want to wait until next week to keep working on this, you can go back to the original solution you posted in the issue (using plain text only). As that is an existing pattern in this codebase, we can use it without needing to gain broader consensus. |
It's fine by me, I can wait here. |
This is good. Yeah, let's wait |
|
I am having some problems when merging main, also the code needs to be change here so I am closing this one and will raise a new PR. |
Details
Used RenderHTML for little refactor and added a new welcome text for deleted #admins/#announce rooms
Fixed Issues
$ #7625
Tests | QA Steps
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNameproperty(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNameproperty(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Screenshots
Web
Mobile Web
Desktop
iOS
Android