CIF-1551 - [React Components] Refactor the NavigationContext #379
Conversation
Remove the NavigationContext and manage the navigation state lower in the tree, in the `AuthBar` component.
Codecov Report
@@ Coverage Diff @@
## master #379 +/- ##
============================================
- Coverage 86.12% 85.77% -0.35%
Complexity 924 924
============================================
Files 187 186 -1
Lines 4813 4780 -33
Branches 673 669 -4
============================================
- Hits 4145 4100 -45
- Misses 531 543 +12
Partials 137 137
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
laurentiumagureanu,
herzog31,
cjelger,
hbacila and
LSantha
| const startAccMgEvent = new CustomEvent(events.START_ACC_MANAGEMENT); | ||
| const exitAccMgEvent = new CustomEvent(events.EXIT_ACC_MANAGEMENT); | ||
|
|
||
| const navigationPanel = document.querySelector('aside.navigation__root'); |
There was a problem hiding this comment.
The selector should be taken from the ConfigContext, so it can be customized.
| @@ -31,6 +31,7 @@ describe.each([ | |||
| }); | |||
|
|
|||
| expect(files.length).toBeGreaterThan(0); // Ensures we read the right folder | |||
| // eslint-disable-next-line no-console | |||
| console.log(`Validating ${files.length} GraphQL requests against Magento schema ${version}`); | |||
There was a problem hiding this comment.
Please remove this line instead of setting an eslint exception.
There was a problem hiding this comment.
It was really useful to have that line in the tests, to see at least that something happens and that multiple versions are being processed.
There was a problem hiding this comment.
@cjelger the Magento version information is already displayed as part of the test description
- read the navigation panel selector from the configuration context - remove the log statement from the `validateQueries` test.
…ents into tasks/CIF-1551
…ents into tasks/CIF-1551
…anged when navigating the "My Account" feature. The bug was caused by a clash of names between the OOTB `dispatchEvent` function and the custom one. Renaming the function and moving it in the same scope as the caller fixed the issue.
Description
A React Context is a useful feature, allowing components to access context data without having the receive it via props. However, this comes with a cost - the components need to be wrapped in the context provider. We must always weigh the pros and cons of using a context and judge on a case by case basis.
In this particular case, the state of the "My Account Panel" (renamed it!) can be managed by the local state without the need of a "Context". I implemented a
useNavigationStatehook that manages all the states of the My Account panel and I removed theNavigationContextand the actions module. Also, I updated theAuthBarcomponent so that it uses the new state management.Related Issue
CIF-1551
Motivation and Context
Reduce code complexity.
How Has This Been Tested?
New unit tests, functional testing.
Screenshots (if appropriate):
Types of changes
Checklist: