-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add Welcome message functionality in room settings #18662
Conversation
Nvm, doing it here |
Only admins or policy owner
Who doesn't see the welcome message, the person you just invited?
This is per chat room, so whatever message the admin sets for the room, it will show the same for everyone. Members can't set any welcome message |
This comment was marked as outdated.
This comment was marked as outdated.
@pecanoro ignore the above bug. I can't reproduce it. I must have been doing something wrong. |
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.
LGTM!
🎯 @rushatgabhane, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #19626. |
@PauloGasparSv All yours! |
Sorry for the delay, will review this today : ) |
✋ 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/PauloGasparSv in version: 1.3.19-0 🚀
|
🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.19-7 🚀
|
|
||
function ReportWelcomeMessagePage(props) { | ||
const parser = new ExpensiMark(); | ||
const [welcomeMessage, setWelcomeMessage] = useState(parser.htmlToMarkdown(props.report.welcomeMessage)); |
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.
Coming from #19688:
Offline case was not handled while implementing welcome message feature. welcomeMessage
value was not added optimistically when build report so app crashed on this line.
<TextInput | ||
inputID="welcomeMessage" | ||
label={props.translate('welcomeMessagePage.welcomeMessage')} | ||
multiline |
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.
Coming from #19698 ,multiline
by default aligns the text to the top on iOS, and centers it on Android. It should be used with textAlignVertical
set to top
for the same behavior in both platforms.
welcomeMessageInputRef.current.focus(); | ||
}} | ||
> | ||
<FullPageNotFoundView shouldShow={_.isEmpty(props.report)}> |
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.
Hi ✋ Coming from #22013
The page is only accessible for policy admin. The check for if report exist is handle from the HOC withReportOrNotFound
.
if (!welcomeMessageInputRef.current) { | ||
return; | ||
} | ||
welcomeMessageInputRef.current.focus(); |
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.
Hi ✋ Coming from #21523
Since the input is multiline and the value can be set from the previous message saved, it makes sense to focus the input and put the cursor on to the end of the text.
}, [props.report.reportID, props.report.welcomeMessage, welcomeMessage]); | ||
|
||
return ( | ||
<ScreenWrapper |
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.
We should have passed:
shouldEnableMaxHeight
to avoid keyboard covering submit button (since we only have one input)includeSafeAreaPaddingBottom={false}
because we have Form that already includes the safe area bottom padding
Coming from #30175
Details
Adds a new item to room settings so admins can set a welcome message in chat rooms.
Monosnap.screencast.2023-05-23.14-42-53.mp4
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/282416
Tests / QA Steps
Create a public room.
Invite a couple of users to the room.
Go to the room settings, you should see a new item to add a welcome message:
Click on the item, it should take you to the following page:
Enter a message with markdown and save it! It should redirect you to the previous page.
Refresh the page, and open the welcome message settings again.
Make sure the welcome message you entered before it's still there.
Save the URL of the room, you will need it for other users to join.
Now log in with a new account that is not part of the room and access the URL of the room.
You should have been invited to the room and a welcome message you saved should show up. It should say that is only visible to you.
Go to the room settings (this user should not be an admin) and make sure the Welcome Message item is not listed:
Log in as one of the users that were previously invited to the room and go to the public room, you should not see the whisper sent to the user that just joined:
Offline tests
Create a public room.
Go to the room settings, you should see a new item to add a welcome message:
Click on the item, it should take you to the following page:
Enter a message with markdown and save it! It should redirect you to the previous page.
Refresh the page, and open the welcome message settings again.
Make sure the welcome message you entered before it's still there.
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
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android