Local nav should be displayed when the first element is dropdown #3465
Conversation
bandana147
requested review from
salonijain3,
sharmrj,
nishantka,
Deva309 and
sonawanesnehal3
as code owners
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -402,13 +402,19 @@ class Gnav { | |||
| `; | |||
| }; | |||
|
|
|||
| removeLocalNav = () => { | |||
There was a problem hiding this comment.
🚫 [eslint] <class-methods-use-this> reported by reviewdog 🐶
Expected 'this' to be used by class method 'removeLocalNav'.
| removeLocalNav = () => { | ||
| lanaLog({ message: 'Gnav Localnav was removed, potential CLS' }); | ||
| document.querySelector('.feds-localnav')?.remove(); | ||
| return; |
There was a problem hiding this comment.
🚫 [eslint] <no-useless-return> reported by reviewdog 🐶
Unnecessary return statement.
| removeLocalNav = () => { | |
| lanaLog({ message: 'Gnav Localnav was removed, potential CLS' }); | |
| document.querySelector('.feds-localnav')?.remove(); | |
| return; | |
| removeLocalNav = () => { | |
| lanaLog({ message: 'Gnav Localnav was removed, potential CLS' }); | |
| document.querySelector('.feds-localnav')?.remove(); | |
| lanaLog({ message: 'Gnav Localnav was removed, potential CLS' }); | ||
| document.querySelector('.feds-localnav')?.remove(); | ||
| return; | ||
| } |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## mobile-gnav #3465 +/- ##
===============================================
+ Coverage 96.46% 98.53% +2.06%
===============================================
Files 256 77 -179
Lines 59987 9871 -50116
===============================================
- Hits 57868 9726 -48142
+ Misses 2119 145 -1974 ☔ View full report in Codecov by Sentry. |
| decorateLocalNav = async () => { | ||
| if (!this.isLocalNav()) return; | ||
| if (this.isLocalNav()) this.removeLocalNav(); |
There was a problem hiding this comment.
should it also return here?
There was a problem hiding this comment.
It returns from removeLocalNav function
| if (!firstElem) { | ||
| lanaLog({ message: 'GNAV: Incorrect authoring of localnav found.', tags: 'errorType=info,module=gnav' }); | ||
| return; | ||
| this.removeLocalNav(); |
There was a problem hiding this comment.
and return here as well? That way it'll be less confusing to read and modify the logic below.
Resolves: MWPW-164828
Test URLs:
QA: https://main--cc--adobecom.hlx.page/drafts/blaishram/document2?milolibs=mgnav-dropdown&newNav=true