Fix highlight of duplicate sub-nav items #836
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #836 +/- ##
=======================================
Coverage 61.05% 61.05%
=======================================
Files 45 45
Lines 873 873
Branches 169 169
=======================================
Hits 533 533
Misses 277 277
Partials 63 63
|
karelhala
requested changes
Jun 10, 2020
There was a problem hiding this comment.
Looking good! Just one smaller improvement so apps can activate secondary items that are outside currently active app. Also would be nice to mention that in docs https://github.com/RedHatInsights/insights-chrome/blob/master/README.md#javascript-api
insights.chrome.appNavClick({id: 'ocp-on-aws', parentId: 'some-parent', secondaryNav: true})
src/js/redux/reducers.js
Outdated
| @@ -54,7 +54,7 @@ export function appNavClick(state, { payload }) { | |||
| globalNav: payload.custom ? state.globalNav && state.globalNav.map(item => ({ | |||
| ...item, | |||
| active: payload && (item.id === payload.id || item.title === payload.id) | |||
| || (item.subItems && item.subItems.some(({ id }) => id === payload.id)) | |||
| || (item.subItems && item.subItems.some(({ id }) => id === payload.id) && item.id === state.appId) | |||
There was a problem hiding this comment.
Can you add payload parent here as well?
Suggested change
| || (item.subItems && item.subItems.some(({ id }) => id === payload.id) && item.id === state.appId) | |
| || (item.subItems && item.subItems.some(({ id }) => id === payload.id) && (item.id === state.appId || item.id === payload.parentId)) |
fhlavac
force-pushed
the
fix_id_highlight
branch
from
83c4b1b
to
0df15b6
Compare
karelhala
approved these changes
Jun 10, 2020
karelhala
pushed a commit
to karelhala/insights-chrome
that referenced
this pull request
Jun 10, 2020
* Fix highlight of duplicate sub-nav items * Add parentId verification to highlight condition * Add appNavClick - parentId param to docs
karelhala
added a commit
that referenced
this pull request
Jun 10, 2020
Fix highlight of duplicate sub-nav items (#836)
ryelo
added a commit
that referenced
this pull request
Jun 11, 2020
* Update all deps (#795) * Update PF and FEC deps to newest stable versions * Update all other deps to latest stable version * Fix linting errors * update case and split logout (#799) Co-authored-by: Karel Hala <[email protected]> * use a skeleton instead of logging in text (#792) * add en-us to automation platform link (#808) per james bailey on the customer portal side, this addition should resolve the issue with doc link in automation platform going to dead page. Co-authored-by: Ryan Long <[email protected]> * Bump @redhat-cloud-services/frontend-components-inventory (#809) * update sentry and add more apps (#802) * update sentry and add more apps * lint * Use direct imports of PF4 components (#814) * Bump @redhat-cloud-services/frontend-components from 1.0.24 to 1.0.28 (#815) * Add Catalog, Approval and Sources to about modal app list. (#834) * Update inv package to newest version 33 (#835) * Fix highlight of duplicate sub-nav items (#836) * Fix highlight of duplicate sub-nav items * Add parentId verification to highlight condition * Add appNavClick - parentId param to docs Co-authored-by: Ryan Long <[email protected]> Co-authored-by: Chris Budzilowicz <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Martin Maroši <[email protected]> Co-authored-by: Filip Hlavac <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix the issue with multiple highlights when the sub-nav item name is already used by another top-level nav item.
Needed for: RedHatInsights/cloud-services-config#218
@karelhala