Closed petesfrench closed 2 months ago
From description it seems that the scope of this PR is parsing yaml and passing data to jinja template, but there is a lot of JS files changed in this PR. Are these changes relevant?
From description it seems that the scope of this PR is parsing yaml and passing data to jinja template, but there is a lot of JS files changed in this PR. Are these changes relevant?
@bartaz I left a note in the description: 'Note this branches of this pr, for clarity it might help to wait for that to be merged before reviewing'
All modified and coverable lines are covered by tests :white_check_mark:
Please upload report for BASE (
canonical-navigation@1d45cbb
). Learn more about missing BASE report.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@britneywwc Could we pair on this work before we review pls? I need your opinion on some things
Thanks for the review :^) 'section_footer' is not used in the mobile view, only desktop view, so can be ignored. The missing section was caused by the typo you flagged. Can you take another look please @britneywwc ?
Done
build_navigation
, that takes and 'id' and 'title' and returns the relevant section on the navigation. It works by parsing the navigation.yaml and applying it to a jinja template.QA
It is a little annoying to QA as the JS that this relies on has not yet been implemented, but do the best you can from the directions below
role="menuitem"
, you should seeli
elements with names like 'product', 'solutions', etc. Within these you will find a<div class="p-navigation__dropdown-content-mobile">
, this is where the navigation generated from this work can be found.Issue / Card
Fixes https://warthogs.atlassian.net/browse/WD-13241