-
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
[WorkspaceInviteMemberPage] update welcome note on workspace name change #17559
Conversation
Signed-off-by: Prince Mendiratta <[email protected]>
@youssef-lr @thesahindia One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-04-19.at.3.07.39.AM.movMobile Web - ChromeScreen.Recording.2023-04-19.at.3.53.04.AM.movMobile Web - SafariScreen.Recording.2023-04-19.at.3.39.54.AM.movDesktopScreen.Recording.2023-04-19.at.3.57.14.AM.moviOSScreen.Recording.2023-04-19.at.3.34.50.AM.movAndroidScreen.Recording.2023-04-19.at.3.47.01.AM.mov |
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.
Works well!
All yours @youssef-lr
✋ 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/youssef-lr in version: 1.3.3-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.3.3-2 🚀
|
Details
With this PR, when the Workspace Invite Member Page is open on one device
and the workspace name is changed from another device, then the welcome
note updates with the new workspace name, if it has not been modified by
the user.
Fixed Issues
$ #16600
PROPOSAL: #16600 (comment)
Tests
1.
device 1.
Offline tests
Although the PR does not modify any functionality depending on network
changes, the expected offline behaviour is:
If either of the 2 devices are offline, then the changes made to the
workspace name would not be reflected.
QA Steps
Same as above.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiondisplays the correct error message if the entered data is not correct)
to ensure it matches the expected behavior (i.e. verify the default avatar
icon is displayed if app is offline)
account
against the staging or production API to ensure there are no regressions
(e.g. long loading states that impact usability).
platforms
not related to the PR, report it or open an issue for it to be fixed)
code)
are named for what the method does and never what callback they handle
(i.e.
toggleReport
and notonIconClick
)explanatory
English, and explained "why" the code was doing something instead of only
explaining "what" the code was doing.
adding it to
src/languages/*
files and using the translationmethod
translation was requested/reviewed in #expensify-open-source and it was
approved by an internal Expensify engineer. Link to Slack message:
in the product are using the localization
methods
English and approved by marketing by adding the
Waiting for Copy
labelfor a copy review on the original GH to get the correct copy.
new files or renamed files. All non-platform specific files are named
after what they export and are not named "index.js". All platform-specific
files are named for the platform the code supports as outlined in the
README.
STYLE.md
)were followed
by multiple Expensify engineers
Guidelines
if the PR modifies a shared library or component like
Avatar
, I verifiedthe components using
Avatar
are working as expected)more than once, with the exception of tests)
CONST.js or at the top of the file that uses the constant) are defined as
such
have also been updated correctly
/** comment above it */
purpose of the component can be inferred from the name alone
rendering and nothing else
event handlers are bound to
this
properly so there are no scoping issues(i.e. for
onClick={this.submit}
the methodthis.submit
should be boundto
this
in the constructor)this
are necessary to be bound(i.e. avoid
this.submit = this.submit.bind(this);
ifthis.submit
isnever passed to a component event handler like
onClick
)purpose, and it is broken down into smaller components in order to
separate concerns and functions
at the top of the file if the code is not self explanatory
StyleUtils
function (i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)those changes do not break usages of that component in the rest of the App
(i.e. if a shared library or component like
Avatar
is modified, Iverified that
Avatar
is working as expected in all cases)Storybook stories, I tested and verified all stories for that component
are still working as expected.
ScrollView
component to make it scrollable when more elements are added to the page.
main
branch was merged into this PR after a review, Itested again and verified the outcome was still expected according to the
Test
steps.including those that don't apply to this PR.
Screenshots/Videos
Web
web.mp4
Mobile Web - Chrome
android-mWeb.mp4
Mobile Web - Safari
ios-mWeb.mp4
Desktop
desktop.mp4
iOS
ios.mp4
Android
android.mp4