CIF-1398 - [My Account] Address book management #353
Conversation
* Add a new component `account` to header, it provides a container div for its react component * Add new `<Account>` component to main React app `react-components.js`, it renders `<AccountTrigger>` component to toggle the `<AccountDropdown>` component, two components `<SignOutLink>` and `<SignInForm>` are created for reusable purpose * Add a new reducer to toggle open state of account dropdown
* Refactor `<SignIn>` and `<MyAccount>` components to use `<SignOutLink>` and `<SignInForm>` created for account dropdown, remove the duplicate CSS as they are existing in CSS file of `<SignInForm>`
* Fix a bug found in `<AccountLink>` component, the issue was that the `identity-obj-proxy` module used by `jest` during the test running will mock `classes` imported to `<AccountLink>`, this breaks the rendering of the `<Button>` component because it doesn't expect `classes` to be mocked, instead, it should be an imported CSS modules, so we assign imported CSS modules to an object `classes`, now the mock won't happen so the test can run correctly * Add or update test files for `<AccountLink>`, `<MyAccount>`, `<SignOutLink>`, `<SignIn>`, and `<SignInForm>`, add generated or updated snapshots files for them as well
* Add a new component `<AccountIconText>` to display the sign in text if the user is not signed in, or display greeting text if the user has signed in, this component will be displayed next to the account icon on header of storefront * Add corresponding translations in `account.json` i18n file
* Fix the prettier issue for `accountIconText.css`
* Add 'package-info.java' to change the version to '2.0.0' as suggested by Java baseline plugin during the mvn build
* Now we can use account dropdown to reset password and create a new account * Add 'Address Book' and 'Address Information' to dropdown, at the moment, they're dummy links, the implementations will be added in other ticket * Add new behaviour, now after user signs in, the account dropdown will be hidden, same behaviour is applied after user signs out * Refactor code so that components and hooks can be reused for account dropdown
* Add tests to verify the components are rendered as expected
…e '<AccountIconText>' component (#303) * Changes the references of account component in other files * Merge logic of '<AccountIconText>' to '<AccountContainer>', now '<AccountTrigger>' will render icon label based on 'label' property passed in * Update test and snapshots of '<AccountContainer>' component
* Remove '<MyAccountLinks>' and '<SignOutLink>' components and move their logic back to '<MyAccount>' component because now we will reuse '<MyAccount>' in account dropdown * Add translation string for cancel button of change password form * Add links of address book and account information to my account so that side panel now will have them available to
* Add a new form title * Move forgot password button to underneath the password field * Change label of email input field * Delete '<SignInForm>' component and merge its logic back into '<SignIn>' as now we are going to reuse it in account drop down and side panel * Update test and snapshots
…oing to use miniaccount component to render the mount point for account dropdown
…ve the unused CSS file (#303)
…reate one React portal in '<AccountContainer>' component, rather than create two React portals in '<AccountDropdown>' and '<AccountTrigger>' respectively (#303)
…to the folder of 'AccountContainer', update the import paths in '<AccountContainer>', this change allows us to group the account dropdown related components in same place (#303)
…ere is sign-in error (#302) * Now we only dispatch 'toggleAccountDropdown' action when the sign-in is successful
* Test if handler function of click event is called when button is clicked
* Remove mock of 'ReactDOM.createPortal' from test suits of account dropdown and account trigger as this method is not used by these two components any more
* Add a new state 'accountContainerQuerySelector' in '<UserContextProvider>' to specify the DOM query selector that should be used to find the account container on header * Add a new state 'authBarContainerQuerySelector' in '<NavigationContextProvider>' to specify the DOM query selector that should be used to find the auth bar container on navigation side panel
Codecov Report
@@ Coverage Diff @@
## master #353 +/- ##
=============================================
+ Coverage 66.67% 84.52% +17.85%
Complexity 852 852
=============================================
Files 192 174 -18
Lines 5845 4400 -1445
Branches 912 587 -325
=============================================
- Hits 3897 3719 -178
+ Misses 1822 555 -1267
Partials 126 126
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
dplaton
requested review from
dplaton,
laurentiumagureanu,
herzog31,
mhaack and
LSantha
| @@ -72,6 +75,10 @@ const NavigationContextProvider = props => { | |||
|
|
|||
| const showAccountCreated = () => NavigationActions.showAccountCreated({ dispatch, t }); | |||
|
|
|||
| const showAddressBook = () => { | |||
| window.location.href = navigationState.addressBookPath; | |||
There was a problem hiding this comment.
Shouldn't we rather use history.push here? See https://reactrouter.com/web/api/Hooks/usehistory.
There was a problem hiding this comment.
currently, the react-router-dom is only installed in venia repo not in core components repo, are you happy with how the router is used in venia repo? I tested in my local, it seems that browser controls history works as well, but I will have a look for sure, thank you @herzog31 :)
There was a problem hiding this comment.
We can install react-router-dom in aem-core-cif-components, but we have to put it as peerDependency. But as long as the feature works without it...
There was a problem hiding this comment.
Browser history should work with window.location.href, just wondering if it has any side effects in terms of navigating within React. Probably only works for navigating between actual AEM pages.
There was a problem hiding this comment.
Hi @herzog31, I tried to use useHistory and then history.push, but since we don't have React router context introduced in core components repo yet, it doesn't seem working at the moment, so are you happy for leaving this logic as is at the moment or do you have other thoughts on this? :)
There was a problem hiding this comment.
If React navigation works with it, let's keep it :)
There was a problem hiding this comment.
Thanks for that @herzog31 :)
…e name of test suite in UserContext.test.js
|
@nettree I quickly check the feature and run into some bugs:
|
|
Hi @dplaton, this is something I'd like to discuss with you, it seems like the endpoints don't accept email as one of the address field, in this case, shall we hide the email field from the address form for address book? I've added the logic to hide the email field by default in address form and display it for address form used in checkout now. :) The issue for the default address has been fixed, it's because the state is not updated properly when there is another address set as default address, but the actual change is applied to BE, when refreshing the page we can only see one default address, I've added the logic to reset corresponding fields when it's necessary to fix that |
… the related tests * When an address is created or updated, the GraphQL endpoints won't accept email as a valid field thus won't save it, so we will hide email input field from address form used in address book now * Explicitly show email input field in address form used in checkout to keep the existing feature working as expected * Update the tests to reflect this changes * Add one test suite to increase the test coverage rate for addressForm.js
…fault address, the previous default address still has the 'Default' label applied
… coverage report
This PR is to add the feature of address book management for the signed in user
Description
User can see the saved address by accessing the address book page
User can create a new address, edit and delete an existing address
User can set an address to their default address(shipping and billing at the moment)
Related Issue
#327
Motivation and Context
This is to enhance the feature set of the user account so that the user can manage their shipping and billing addresses when they've signed in.
How Has This Been Tested?
All the changes made have jest tests covered, features are tested in the browser, including create, edit, delete the address, and set the default address
Screenshots (if appropriate):
Types of changes
Checklist: