-
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
adding the Expensify Chat article #12153
Conversation
Gotta update the formatting. I'll work on that in a sec. |
Hello, |
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.
Looks good to me so far! Would be great to have the test steps outlined here.
In addition, do we need to have this tested on all platforms? cc: @marcochavezf Considering this is just an update to help docs, I'm not sure we need a C+ review?
Ah yup, C+ review is not needed. @johncschuster we also need to add the links to the LHN and hub page. Rory yesterday implemented yesterday something to add the links to the LHN in an easy way (no need to update HTML), I'm going to implement something similar to the hub pages. I will let you know when it's ready, so we can add the links to this article in this PR. |
Sounds great, @marcochavezf! Thanks! |
Update: added the article link to the LHN and Send money hub, it's ready to be reviewed. @johncschuster let me know if the article looks good in the videos :) |
|
Merging to push the article to the site |
✋ 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 @marcochavezf in version: 1.2.25-0 🚀
|
🚀 Deployed to production by @yuwenmemon in version: 1.2.25-0 🚀
|
Details
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/234158
Tests
Pre-requisites (only in dev):
In order to run the site locally you would need ruby and bundler installed in your machine, more info here.
Then just execute the following commands in your command-line:
Steps:
QA Steps
Same steps as
Test
section (test only in production).PR Review Checklist
PR Author Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesWaiting 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)PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesWaiting 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
have been tested & I retested again)/** 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)Screenshots
Web
Screen.Recording.2022-11-03.at.20.26.49.mov
Mobile Web - Chrome
Screen.Recording.2022-11-03.at.20.49.56.mov
Mobile Web - Safari
Screen.Recording.2022-11-03.at.20.50.52.mov