-
Notifications
You must be signed in to change notification settings - Fork 22
fix issue3440 #111
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
base: nav-bugbash-2
Are you sure you want to change the base?
fix issue3440 #111
Conversation
@@ -22,7 +22,7 @@ const SubNav = ({ | |||
const to = _.isEmpty(level3.link) ? level3.href : level3.link | |||
return ( | |||
<Link | |||
className={cn(styles.secondaryNavItem, level3.id === activeChildId && styles.secondaryNavItemOpen)} | |||
className={cn(styles.secondaryNavItem, level3.id === activeChildId && styles.secondaryNavItemOpen, level3.title === 'All Challenges' && styles.allChallengeClickable)} |
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.
@LieutenantRoger This not a valid solution, because this is hardcoded if we change the menu to any other sentence this fix will stop working.
@@ -72,6 +72,10 @@ | |||
@include Roboto-Bold; | |||
@include not-clickable; | |||
} | |||
&.allChallengeClickable { |
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.
Also just add CSS class to specific menu name not valid.
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.
Please check comments in code.
Also solution need be done in logic of navigation, try check this section:
src/components/TopNav/index.js
line: 435
This method can't detect the change of the query parameter in the url, this is the reason causing this bug. Please advise otherwise way, maybe passing the query parameter into the TopNav component as well ? Or we can just allow user to re-click/re-enter the same level3 menu item, it doesn't have negative user experience as well IMO. |
I'll get confirmation about this and back to you. |
@LieutenantRoger Please follow this way:
|
No description provided.