From 9baf37a75bcb810a837cceca55eea8be13726f10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Souk=C3=A9ina=20Bojabza?= Date: Fri, 15 Dec 2023 14:53:18 +0100 Subject: [PATCH] [FIX] website, website_blog, *: fix duplicated navbar ids *: test_website_modules In PR [1], the headers have been redesigned. One of their major changes is that now, they have two navbars: one for the desktop view and one for the mobile view. However, the `o_main_nav` and `top_menu` navbar ids are also duplicated, meaning that there are multiple elements with the same id in the DOM, which is not a good practice. This commit fixes this by replacing the ids by classes in the navbar templates, since their use does not really require for them to be ids (e.g. `#o_main_nav` is mainly for styling). For the desktop view, the ids are still kept (for stability purpose). Note that the CSS rules and the selectors in the JS files (except for the tests) now take into account both the ids and the classes (to be safe). [1]: https://github.com/odoo/odoo/pull/119650 task-3609531 Part-of: odoo/odoo#146492 --- .../static/tests/tours/configurator_flow.js | 4 +- .../wysiwyg_adapter/wysiwyg_adapter.js | 3 +- .../static/src/js/content/auto_hide_menu.js | 3 +- .../website/static/src/js/tours/tour_utils.js | 2 +- addons/website/static/src/scss/website.scss | 48 +++++++++++++------ .../static/tests/tours/edit_link_popover.js | 6 +-- .../static/tests/tours/edit_megamenu.js | 12 ++--- .../website/static/tests/tours/edit_menus.js | 42 ++++++++-------- .../static/tests/tours/website_navbar_menu.js | 2 +- addons/website/views/snippets/snippets.xml | 1 + addons/website/views/website_templates.xml | 30 +++++++----- .../static/tests/tours/blog_tags_tour.js | 2 +- 12 files changed, 91 insertions(+), 64 deletions(-) diff --git a/addons/test_website_modules/static/tests/tours/configurator_flow.js b/addons/test_website_modules/static/tests/tours/configurator_flow.js index 65707c04652..81274c2cb9a 100644 --- a/addons/test_website_modules/static/tests/tours/configurator_flow.js +++ b/addons/test_website_modules/static/tests/tours/configurator_flow.js @@ -81,12 +81,12 @@ registry.category("web_tour.tours").add('configurator_flow', { run: function () { const $iframe = this.$anchor.find('iframe.o_iframe:not(.o_ignore_in_tour)'); for (const menu of ['Home', 'Events', 'Courses', 'Pricing', 'News', 'Success Stories', 'Contact us']) { - if (!$iframe.contents().find(`#top_menu a:contains(${menu})`).length) { + if (!$iframe.contents().find(`.top_menu a:contains(${menu})`).length) { console.error(`Missing ${menu} menu. It should have been created by the configurator.`); } } for (const url of ['/', '/event', '/slides', '/pricing', '/blog/', '/blog/', '/contactus']) { - if (!$iframe.contents().find(`#top_menu a[href^='${url}']`).length) { + if (!$iframe.contents().find(`.top_menu a[href^='${url}']`).length) { console.error(`Missing ${url} menu URL. It should have been created by the configurator.`); } } diff --git a/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js b/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js index dc27b31ef6a..20a86f16a35 100644 --- a/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js +++ b/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js @@ -166,7 +166,8 @@ export class WysiwygAdapterComponent extends Wysiwyg { const $editableWindow = this.$editable[0].ownerDocument.defaultView; // Dropdown menu initialization: handle dropdown openings by hand - var $dropdownMenuToggles = $editableWindow.$('.o_mega_menu_toggle, #o_main_nav .dropdown-toggle'); + // TODO in master: remove the selector with the `#o_main_nav` id. + const $dropdownMenuToggles = $editableWindow.$(".o_mega_menu_toggle, #o_main_nav .dropdown-toggle, .o_main_nav .dropdown-toggle"); $dropdownMenuToggles.removeAttr('data-bs-toggle').dropdown('dispose'); // Since bootstrap 5.1.3, removing bsToggle is not sufficient anymore. $dropdownMenuToggles.siblings(".dropdown-menu").addClass("o_wysiwyg_submenu"); diff --git a/addons/website/static/src/js/content/auto_hide_menu.js b/addons/website/static/src/js/content/auto_hide_menu.js index 5afdc23d0b1..cfffa333ca3 100644 --- a/addons/website/static/src/js/content/auto_hide_menu.js +++ b/addons/website/static/src/js/content/auto_hide_menu.js @@ -275,7 +275,8 @@ async function autoHideMenu(el, options) { document.addEventListener('DOMContentLoaded', async () => { const header = document.querySelector('header#top'); if (header) { - const topMenu = header.querySelector('#top_menu'); + // TODO in master: remove `#top_menu` from the selector. + const topMenu = header.querySelector("#top_menu, .top_menu"); if (header.classList.contains('o_no_autohide_menu')) { topMenu.classList.remove('o_menu_loading'); return; diff --git a/addons/website/static/src/js/tours/tour_utils.js b/addons/website/static/src/js/tours/tour_utils.js index 19fe57e05fd..1007d1cee93 100644 --- a/addons/website/static/src/js/tours/tour_utils.js +++ b/addons/website/static/src/js/tours/tour_utils.js @@ -303,7 +303,7 @@ function getClientActionUrl(path, edition) { function clickOnExtraMenuItem(stepOptions, backend = false) { return Object.assign({}, { content: "Click on the extra menu dropdown toggle if it is there", - trigger: `${backend ? "iframe" : ""} #top_menu`, + trigger: `${backend ? "iframe" : ""} .top_menu`, run: function () { const extraMenuButton = this.$anchor[0].querySelector('.o_extra_menu_items a.nav-link'); if (extraMenuButton) { diff --git a/addons/website/static/src/scss/website.scss b/addons/website/static/src/scss/website.scss index 01f5e3a88b5..685e239c72a 100644 --- a/addons/website/static/src/scss/website.scss +++ b/addons/website/static/src/scss/website.scss @@ -297,7 +297,8 @@ $-seen-urls: (); .o_menu_image_placeholder { width: 80px !important; } - #top_menu { + // TODO in master: remove the `#top_menu` selector. + #top_menu, .top_menu { flex-wrap: nowrap !important; &.o_menu_loading { overflow: hidden !important; @@ -1034,7 +1035,8 @@ header { z-index: $zindex-website-header; } &:not(.o_header_no_transition) { - #o_main_nav { + // TODO in master: remove the `#o_main_nav` selector. + #o_main_nav, .o_main_nav { transition: all $-transition-duration, color 0ms; } .navbar-brand { @@ -1073,7 +1075,8 @@ header { $-is-hamburger: o-website-value('header-template') == 'hamburger'; @include media-breakpoint-up(md) { @if $-is-hamburger { - #o_main_nav { + // TODO in master: remove the `#o_main_nav` selector. + #o_main_nav, .o_main_nav { padding-top: $spacer * 0.5; padding-bottom: $spacer * 0.5; } @@ -1081,7 +1084,8 @@ header { } } - #o_main_nav { + // TODO in master: remove the `#o_main_nav` selector. + #o_main_nav, .o_main_nav { flex-direction: inherit; } @@ -1141,7 +1145,8 @@ header { max-width: 100%; padding: 0 0 $spacer 0; } - #o_main_nav { + // TODO in master: remove the `#o_main_nav` selector. + #o_main_nav, .o_main_nav { flex-direction: column; align-items: start; padding: 0; @@ -1180,9 +1185,14 @@ header { @include o-apply-colors('header-sales_one-custom'); @include o-add-gradient('menu-secondary-gradient'); } - header#top > nav:not(.o_header_mobile) #o_main_nav > .container { - // TODO: remove in master and add "align-items-center" in the XML. - align-items: center; + header#top > nav:not(.o_header_mobile) { + // TODO in master: remove the `#o_main_nav` selector. + #o_main_nav,.o_main_nav { + > .container { + // TODO: remove in master and add "align-items-center" in the XML. + align-items: center; + } + } } } @else if o-website-value('header-template') == 'sales_two' { #wrapwrap .o_header_sales_two_top { @@ -1190,9 +1200,14 @@ header { @include o-apply-colors('header-sales_two-custom'); @include o-add-gradient('menu-secondary-gradient'); } - header#top > nav:not(.o_header_mobile) #o_main_nav #top_menu { - // TODO: remove in master and add "align-items-center" in the XML. - align-items: center; + header#top > nav:not(.o_header_mobile) { + // TODO in master: remove the `#o_main_nav` and `#top_menu` selectors. + #o_main_nav, .o_main_nav { + #top_menu, .top_menu { + // TODO: remove in master and add "align-items-center" in the XML. + align-items: center; + } + } } } @else if o-website-value('header-template') == 'sales_three' { #wrapwrap .o_header_sales_three_top { @@ -1206,9 +1221,14 @@ header { @include o-apply-colors('header-sales_four-custom'); @include o-add-gradient('menu-secondary-gradient'); } - header#top > nav:not(.o_header_mobile) #o_main_nav .navbar-nav { - // TODO: remove in master and add "align-items-center" in the XML. - align-items: center; + header#top > nav:not(.o_header_mobile) { + // TODO in master: remove the `#o_main_nav` selector. + #o_main_nav, .o_main_nav { + .navbar-nav { + // TODO: remove in master and add "align-items-center" in the XML. + align-items: center; + } + } } } diff --git a/addons/website/static/tests/tours/edit_link_popover.js b/addons/website/static/tests/tours/edit_link_popover.js index ce940832184..9a73e2e8ea0 100644 --- a/addons/website/static/tests/tours/edit_link_popover.js +++ b/addons/website/static/tests/tours/edit_link_popover.js @@ -92,7 +92,7 @@ wTourUtils.registerWebsitePreviewTour('edit_link_popover', { // 2. Test links in navbar (website) { content: "Click navbar menu Home", - trigger: 'iframe #top_menu a:contains("Home")', + trigger: 'iframe .top_menu a:contains("Home")', }, { content: "Popover should be shown (2)", @@ -112,7 +112,7 @@ wTourUtils.registerWebsitePreviewTour('edit_link_popover', { { content: "Click on the Home menu again", extra_trigger: 'div:not(.o_loading_dummy) > #oe_snippets', - trigger: 'iframe #top_menu a:contains("Home")[href="/contactus"]', + trigger: 'iframe .top_menu a:contains("Home")[href="/contactus"]', }, { content: "Popover should be shown with updated preview data (2)", @@ -135,7 +135,7 @@ wTourUtils.registerWebsitePreviewTour('edit_link_popover', { // 3. Test other links (CTA in navbar & links in footer) { content: "Click CTA in navbar", - trigger: 'iframe #o_main_nav a.btn-primary[href="/contactus"]', + trigger: 'iframe .o_main_nav a.btn-primary[href="/contactus"]', }, { content: "Popover should be shown (3)", diff --git a/addons/website/static/tests/tours/edit_megamenu.js b/addons/website/static/tests/tours/edit_megamenu.js index 91f0716637c..ec40cea3de3 100644 --- a/addons/website/static/tests/tours/edit_megamenu.js +++ b/addons/website/static/tests/tours/edit_megamenu.js @@ -4,7 +4,7 @@ import wTourUtils from "@website/js/tours/tour_utils"; const toggleMegaMenu = (stepOptions) => Object.assign({}, { content: "Toggles the mega menu.", - trigger: 'iframe #top_menu .nav-item a.o_mega_menu_toggle', + trigger: "iframe .top_menu .nav-item a.o_mega_menu_toggle", run: function () { // If the mega menu is displayed inside the extra menu items, it should // already be displayed. @@ -22,7 +22,7 @@ wTourUtils.registerWebsitePreviewTour('edit_megamenu', { // Add a megamenu item to the top menu. { content: "Click on a menu item", - trigger: 'iframe #top_menu .nav-item a', + trigger: "iframe .top_menu .nav-item a", }, { content: "Click on 'Link' to open Link Dialog", @@ -53,7 +53,7 @@ wTourUtils.registerWebsitePreviewTour('edit_megamenu', { }, // Edit a menu item wTourUtils.clickOnExtraMenuItem({extra_trigger: ".o_website_preview.editor_enable.editor_has_snippets:not(.o_is_blocked)"}, true), - toggleMegaMenu({extra_trigger: 'iframe #top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")'}), + toggleMegaMenu({extra_trigger: 'iframe .top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")'}), { content: "Select the last menu link of the first column", trigger: 'iframe .s_mega_menu_odoo_menu .row > div:first-child .nav > :nth-child(6)', // 6th is the last one @@ -110,7 +110,7 @@ wTourUtils.registerWebsitePreviewTour('edit_megamenu_big_icons_subtitles', { // Add a megamenu item to the top menu. { content: "Click on a menu item", - trigger: 'iframe #top_menu .nav-item a', + trigger: 'iframe .top_menu .nav-item a', }, { content: "Click on 'Link' to open Link Dialog", @@ -141,12 +141,12 @@ wTourUtils.registerWebsitePreviewTour('edit_megamenu_big_icons_subtitles', { }, { content: "Check for the new mega menu", - trigger: 'iframe #top_menu:has(.nav-item a.o_mega_menu_toggle:contains("Megaaaaa2!"))', + trigger: 'iframe .top_menu:has(.nav-item a.o_mega_menu_toggle:contains("Megaaaaa2!"))', run: function () {}, // it's a check }, // Edit a menu item wTourUtils.clickOnExtraMenuItem({extra_trigger: ".o_website_preview.editor_enable.editor_has_snippets:not(.o_is_blocked)"}, true), - toggleMegaMenu({extra_trigger: 'iframe #top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa2!")'}), + toggleMegaMenu({extra_trigger: 'iframe .top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa2!")'}), { content: "Select the first menu link of the first column", trigger: 'iframe .s_mega_menu_odoo_menu .row > div:first-child .nav > :first-child', diff --git a/addons/website/static/tests/tours/edit_menus.js b/addons/website/static/tests/tours/edit_menus.js index 101c8e39d69..49d5a72be56 100644 --- a/addons/website/static/tests/tours/edit_menus.js +++ b/addons/website/static/tests/tours/edit_menus.js @@ -38,14 +38,14 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { wTourUtils.clickOnExtraMenuItem({extra_trigger: 'body:not(:has(.oe_menu_editor))'}, true), { content: "There should be a new megamenu item.", - trigger: 'iframe #top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")', + trigger: 'iframe .top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")', run: () => {}, // It's a check. }, // Add a menu item in edit mode. ...wTourUtils.clickOnEditAndWaitEditMode(), { content: "Click on a menu item", - trigger: 'iframe #top_menu .nav-item a', + trigger: "iframe .top_menu .nav-item a", }, { content: "Click on Edit Menu", @@ -90,7 +90,7 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { wTourUtils.clickOnExtraMenuItem({extra_trigger: '#oe_snippets.o_loaded'}, true), { content: "Menu should have a new link item", - trigger: 'iframe #top_menu .nav-item a:contains("Random!")', + trigger: 'iframe .top_menu .nav-item a:contains("Random!")', // Don't click the new menu when the editor is still blocked. extra_trigger: ".o_website_preview.editor_enable.editor_has_snippets:not(.o_is_blocked)", }, @@ -111,14 +111,14 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { wTourUtils.clickOnExtraMenuItem({}, true), { content: "Label should have changed", - trigger: 'iframe #top_menu .nav-item a:contains("Modnar")', + trigger: 'iframe .top_menu .nav-item a:contains("Modnar")', run: () => {}, // it's a check }, // Edit the menu item from the "edit menu" popover button ...wTourUtils.clickOnEditAndWaitEditMode(), { content: "Click on the 'Modnar' link", - trigger: 'iframe #top_menu .nav-item a:contains("Modnar")', + trigger: 'iframe .top_menu .nav-item a:contains("Modnar")', }, { content: "Click on the popover Edit Menu button", @@ -152,7 +152,7 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { wTourUtils.clickOnExtraMenuItem({}, true), { content: "Label should have changed", - trigger: 'iframe #top_menu .nav-item a:contains("Modnar !!")', + trigger: 'iframe .top_menu .nav-item a:contains("Modnar !!")', run: () => {}, // It's a check. }, // Nest menu item from the menu. @@ -205,13 +205,13 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { }, { content: "Menu item should have a child", - trigger: 'iframe #top_menu .nav-item a.dropdown-toggle:contains("Home")', + trigger: 'iframe .top_menu .nav-item a.dropdown-toggle:contains("Home")', }, // Check that with the auto close of dropdown menus, the dropdowns remain // openable. { content: "When menu item is opened, child item must appear in the shown menu", - trigger: 'iframe #top_menu .nav-item:contains("Home") ul.show li a.dropdown-item:contains("Contact us")[href="/contactus"]', + trigger: 'iframe .top_menu .nav-item:contains("Home") ul.show li a.dropdown-item:contains("Contact us")[href="/contactus"]', run: function () { // Scroll down. this.$anchor[0].closest('body').querySelector('.o_footer_copyright_name') @@ -220,35 +220,35 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { }, { content: "The Home menu should be closed", - trigger: 'iframe #top_menu .nav-item:contains("Home"):has(ul:not(.show))', + trigger: 'iframe .top_menu .nav-item:contains("Home"):has(ul:not(.show))', run: () => {}, // It's a check. }, { content: "Open the Home menu after scroll", - trigger: 'iframe #top_menu .nav-item a.dropdown-toggle:contains("Home")', + trigger: 'iframe .top_menu .nav-item a.dropdown-toggle:contains("Home")', }, { content: "Check that the Home menu is opened", - trigger: 'iframe #top_menu .nav-item:contains("Home") ul.show li' + + trigger: 'iframe .top_menu .nav-item:contains("Home") ul.show li' + ' a.dropdown-item:contains("Contact us")[href="/contactus"]', run: () => {}, // It's a check. }, { content: "Close the Home menu", - trigger: 'iframe #top_menu .nav-item:has(a.dropdown-toggle:contains("Home"))', + trigger: 'iframe .top_menu .nav-item:has(a.dropdown-toggle:contains("Home"))', }, { content: "Check that the Home menu is closed", - trigger: 'iframe #top_menu .nav-item:contains("Home"):has(ul:not(.show))', + trigger: 'iframe .top_menu .nav-item:contains("Home"):has(ul:not(.show))', run: () => {}, // It's a check. }, { content: "Open the mega menu", - trigger: 'iframe #top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")', + trigger: 'iframe .top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")', }, { content: "When the mega menu is opened, scroll up", - trigger: 'iframe #top_menu .o_mega_menu_toggle.show', + trigger: "iframe .top_menu .o_mega_menu_toggle.show", run: function () { const marginTopOfMegaMenu = getComputedStyle( this.$anchor[0].closest('.dropdown').querySelector('.o_mega_menu'))['margin-top']; @@ -262,28 +262,28 @@ wTourUtils.registerWebsitePreviewTour('edit_menus', { }, { content: "Check that the mega menu is closed", - trigger: 'iframe #top_menu .nav-item:contains("Megaaaaa!"):has(div[data-name="Mega Menu"]:not(.show))', - run:() => {}, // It's a check. + trigger: 'iframe .top_menu .nav-item:contains("Megaaaaa!"):has(div[data-name="Mega Menu"]:not(.show))', + run: () => {}, // It's a check. }, { content: "Open the mega menu after scroll", - trigger: 'iframe #top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")', + trigger: 'iframe .top_menu .nav-item a.o_mega_menu_toggle:contains("Megaaaaa!")', }, { content: "Check that the mega menu is opened", - trigger: 'iframe #top_menu .nav-item:has(a.o_mega_menu_toggle:contains("Megaaaaa!")) ' + + trigger: 'iframe .top_menu .nav-item:has(a.o_mega_menu_toggle:contains("Megaaaaa!")) ' + '.s_mega_menu_odoo_menu', run: () => {}, // It's a check. }, ...wTourUtils.clickOnEditAndWaitEditMode(), { content: "Open nested menu item", - trigger: 'iframe #top_menu .nav-item:contains("Home"):nth(1) .dropdown-toggle', + trigger: 'iframe .top_menu .nav-item:contains("Home"):nth(1) .dropdown-toggle', }, { // If this step fails, it means that a patch inside bootstrap was lost. content: "Press the 'down arrow' key.", - trigger: 'iframe #top_menu .nav-item:contains("Home") li:contains("Contact us")', + trigger: 'iframe .top_menu .nav-item:contains("Home") li:contains("Contact us")', run: function (actions) { this.$anchor[0].dispatchEvent(new window.KeyboardEvent("keydown", { key: "ArrowDown" })); }, diff --git a/addons/website/static/tests/tours/website_navbar_menu.js b/addons/website/static/tests/tours/website_navbar_menu.js index 7a05aa2592b..a570aa09c05 100644 --- a/addons/website/static/tests/tours/website_navbar_menu.js +++ b/addons/website/static/tests/tours/website_navbar_menu.js @@ -8,7 +8,7 @@ registry.category("web_tour.tours").add("website_navbar_menu", { steps: () => [ { content: "Ensure menus are in DOM", - trigger: '#top_menu .nav-item a:contains("Test Tour Menu")', + trigger: '.top_menu .nav-item a:contains("Test Tour Menu")', run: function () {}, // it's a check }, { content: "Ensure menus loading is done (so they are actually visible)", diff --git a/addons/website/views/snippets/snippets.xml b/addons/website/views/snippets/snippets.xml index 3426d31131e..d90dadfa2c4 100644 --- a/addons/website/views/snippets/snippets.xml +++ b/addons/website/views/snippets/snippets.xml @@ -737,6 +737,7 @@ data-no-preview="true"/> +
+ @@ -298,13 +299,14 @@ + +