MWPW-157147: Dynamic Nav Status Button #2920
Conversation
… QA in understanding which nav is active
JasonHowellSlavin
requested review from
salonijain3,
bandana147,
sharmrj,
nishantka,
Deva309,
sonawanesnehal3 and
a team
as code owners
JasonHowellSlavin
added
run-nala
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2920 +/- ##
==========================================
+ Coverage 96.24% 96.35% +0.10%
==========================================
Files 236 244 +8
Lines 54254 55425 +1171
==========================================
+ Hits 52217 53402 +1185
+ Misses 2037 2023 -14 ☔ View full report in Codecov by Sentry. |
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
…d to disable status. Updated tests
JasonHowellSlavin
force-pushed
the
dynamic-nav-status-button
branch
from
09b27b9
to
753280a
Compare
|
|
This PR is currently in the |
| }; | ||
|
|
||
| const returnPath = (url) => { | ||
| if (!url.includes('https://')) return ''; |
There was a problem hiding this comment.
Probably doesn't matter, but should startsWith be used instead?
There was a problem hiding this comment.
this benchmark says includes is faster, but startsWith is more semantic so I will go with that!
Dli3
added
verified
|
Skipped 2920: "MWPW-157147: Dynamic Nav Status Button" due to file "libs/blocks/global-navigation/global-navigation.js" overlap. Merging will be attempted in the next batch |
|
Skipped 2920: "MWPW-157147: Dynamic Nav Status Button" due to file "libs/utils/utils.js" overlap. Merging will be attempted in the next batch |
1 similar comment
|
Skipped 2920: "MWPW-157147: Dynamic Nav Status Button" due to file "libs/utils/utils.js" overlap. Merging will be attempted in the next batch |
Context:
The milo repo does not have the dynamicNavKey in the config, so the first two links will not show the feature.
Resolves: MWPW-157147
Test URLs:
Bacom