App
App copied to clipboard
Update account settings/profile
Details
Fixed Issues
$ https://github.com/Expensify/App/issues/35603 PROPOSAL: N/A
Tests
- [x] Verify that no errors appear in the JS console
- Log in to the app
- Open Account Settings
- Choose "Profile" if not selected
- Check if everything looks and works fine
Offline tests
QA Steps
- [x] Verify that no errors appear in the JS console
PR Author Checklist
- [x] I linked the correct issue in the
### Fixed Issues
section above - [x] I wrote clear testing steps that cover the changes made in this PR
- [x] I added steps for local testing in the
Tests
section - [x] I added steps for the expected offline behavior in the
Offline steps
section - [x] I added steps for Staging and/or Production testing in the
QA steps
section - [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
- [x] I added steps for local testing in the
- [x] I included screenshots or videos for tests on all platforms
- [x] I ran the tests on all platforms & verified they passed on:
- [x] Android: Native
- [x] Android: mWeb Chrome
- [x] iOS: Native
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
- [x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
- [x] I followed proper code patterns (see Reviewing the code)
- [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e.
toggleReport
and notonIconClick
) - [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g.
myBool && <MyComponent />
. - [x] I verified that comments were added to code that is not self explanatory
- [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- [x] I verified any copy / text shown in the product is localized by adding it to
src/languages/*
files and using the translation method- [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the
Waiting for Copy
label for a copy review on the original GH to get the correct copy. - [x] I verified proper file naming conventions were followed for any 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.
- [x] I verified the JSDocs style guidelines (in
STYLE.md
) were followed
- [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e.
- [x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
- [x] I followed the guidelines as stated in the Review Guidelines
- [x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like
Avatar
, I verified the components usingAvatar
are working as expected) - [x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
- [x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
- [x] I verified that if a function's arguments changed that all usages have also been updated correctly
- [x] If any new file was added I verified that:
- [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
- [x] If a new CSS style is added I verified that:
- [x] A similar style doesn't already exist
- [x] The style can't be created with an existing StyleUtils function (i.e.
StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)
- [x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
- [x] If the PR modifies a generic component, I tested and verified that 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, I verified thatAvatar
is working as expected in all cases) - [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
- [x] If the PR modifies the form input styles:
- [x] I verified that all the inputs inside a form are aligned with each other.
- [x] I added
Design
label so the design team can review the changes.
- [x] If a new page is added, I verified it's using the
ScrollView
component to make it scrollable when more elements are added to the page. - [x] If the
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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
@robertjchen Please 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]
If this requires C+ review, I can help
If this requires C+ review, I can help
cc @mountiny, as this PR blocks other design changes
Sorry for the ping @situchan go ahead please
Lets please make sure everything works as expected too @situchan
please merge main
@situchan Hello! Whats your ETA once this is synced up with main? thank you!
@situchan Hello! Whats your ETA once this is synced up with main? thank you!
going to review immediately.
@filip-solecki please also fix typescript checks
Ready for review! I've pushed the wrong commit here by mistake, that's why we have one revert.
@situchan - ready to review!
I assume old personal details page is being deprecated in this PR? If so, we should update back button behavior. Or if it's out of scope, looks good!
https://github.com/Expensify/App/assets/108292595/429ddb5d-8f07-442b-a918-29cbc7c2db6d
I still think above is considered as regression. Let's fix them here
@situchan I think you're right. Back Button behavior updated and also redirects after updating fields
@filip-solecki Reassure Performance still failing. I remember this was fixed on main and should not happen anymore
@situchan It is all good right now!
How about removing all personal-details
usages throughout the app?
i.e. this is still remaining in NAVIGATION.md
@situchan Hmm, I think it is not only to remove this from md (probably the last place I think) but maybe to replace this example with some other one.
@situchan Hmm, I think it is not only to remove this from md (probably the last place I think) but maybe to replace this example with some other one.
yes correct. need alternative example.
https://dev.new.expensify.com:8082/settings/profile/personal-details/legal-name
And I am not sure /personal-details
path is needed anymore in the middle
https://dev.new.expensify.com:8082/settings/profile/legal-name makes more sense to me
I agree but probably we need some confirmation cc @mountiny WDYT?
Yeah I agree that this new page probably act as the personal-details
url
And what about NAVIGATION.md
? What should we include there instead of current example with /personal-details
url?
And what about
NAVIGATION.md
? What should we include there instead of current example with/personal-details
url?
I guess we could do the address and country page or something else that has multiple layers in the RHP
@situchan - I've commited new changes with removing personal-details
from all routes and I left it only in one place - src/hooks/usePrivatePersonalDetails.ts
- I am not sure if changing names in src/libs/actions/PersonalDetails.ts
will not need some backend work - can you confirm?
I've renamed all consts and routes - left only in src/libs/API/types.ts
as I guess it could need some backend changes. Are you ok with that @mountiny ?
@filip-solecki we are good here, right?
@situchan what is your ETA for this one?
@filip-solecki we are good here, right?
@situchan what is your ETA for this one?
Yes, we are good, waiting for @situchan
on it
Not sure what's happening. Lots of checks are failing
Not sure what's happening. Lots of checks are failing
Looks like it is broken on main.
https://github.com/Expensify/App/pull/35972 should fix it.