From 2658d29e0303482e2fa01cf28cd9ab507a28d091 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 10 Sep 2021 12:02:42 +0000 Subject: [PATCH] [FIX] web_editor, website: re-introduce link double click It was previously decided that double clicking on a link shouldn't do anything other than opening the popover (which happen on single click). It was recently decided that we should actually open the right panel link tool on double click. Adding double click behavior, it was needed to refactor the way the popover is opening/closing -> We now use `focus` as trigger instead of `click`. Side effect, it will also fix 2641448 (part about double click on word) Courtesy of SAD for debugging the double click issue (with trigger click) task-2618494 closes odoo/odoo#76372 X-original-commit: 4eff35ab31bfc45b4c8dca8392e9da4f4e2a7afd Signed-off-by: Romain Derie (rde) Signed-off-by: Quentin Smetz (qsm) --- .../js/wysiwyg/widgets/link_popover_widget.js | 10 +- .../static/src/js/wysiwyg/wysiwyg.js | 23 +++-- .../website/static/src/js/editor/wysiwyg.js | 8 +- .../static/tests/tours/edit_link_popover.js | 97 +++++++++++++------ 4 files changed, 89 insertions(+), 49 deletions(-) diff --git a/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js b/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js index 415ae197829..1f32cab698a 100644 --- a/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js +++ b/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js @@ -53,7 +53,13 @@ const LinkPopoverWidget = Widget.extend({ html: true, content: this.$el, placement: 'bottom', - trigger: 'click', + // We need the popover to: + // 1. Open when the link is clicked or double clicked + // 2. Remain open when the link is clicked again (which `trigger: 'click'` is not doing) + // 3. Remain open when the popover content is clicked.. + // 4. ..except if it the click was on a button of the popover content + // 5. Close when the user click somewhere on the page (not being the link or the popover content) + trigger: 'focus', boundary: 'viewport', }) .on('show.bs.popover.link_popover', () => { @@ -162,7 +168,7 @@ const LinkPopoverWidget = Widget.extend({ /** * Opens the Link Dialog. * - * TODO Call business methods once new editor is released instead of click + * TODO The editor instance should be reached a proper way * * @private * @param {Event} ev diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 43f310890c7..b5db66ca0b5 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -168,17 +168,16 @@ const Wysiwyg = Widget.extend({ self.openMediaDialog(params); }); - if (!this.options.preventLinkDoubleClick) { - this.$editable.on('dblclick', 'a', function () { - if (!this.getAttribute('data-oe-model') && self.toolbar.$el.is(':visible')) { - self.showTooltip = false; - self.toggleLinkTools({ - forceOpen: true, - link: this, - }); - } - }); - } + this.$editable.on('dblclick', 'a', function (ev) { + if (!this.getAttribute('data-oe-model') && self.toolbar.$el.is(':visible')) { + self.showTooltip = false; + self.toggleLinkTools({ + forceOpen: true, + link: this, + noFocusUrl: $(ev.target).data('popover-widget-initialized'), + }); + } + }); if (options.snippets) { $(this.odooEditor.document.body).addClass('editor_enable'); @@ -1294,7 +1293,7 @@ const Wysiwyg = Widget.extend({ this._updateFaResizeButtons(); } const link = getInSelection(this.odooEditor.document, 'a'); - if (isInMedia || link && !this.options.preventLinkDoubleClick) { + if (isInMedia || link) { // Handle the media/link's tooltip. this.showTooltip = true; setTimeout(() => { diff --git a/addons/website/static/src/js/editor/wysiwyg.js b/addons/website/static/src/js/editor/wysiwyg.js index 3d9a1824f5c..97d8fe04142 100644 --- a/addons/website/static/src/js/editor/wysiwyg.js +++ b/addons/website/static/src/js/editor/wysiwyg.js @@ -50,15 +50,13 @@ Wysiwyg.include({ */ start: function () { this.options.toolbarHandler = $('#web_editor-top-edit'); - this.options.preventLinkDoubleClick = true; - $(document.body).on('mousedown', (ev) => { const $target = $(ev.target); + // Keep popover open if clicked inside it, but not on a button - if (!($target.parents('.o_edit_menu_popover').length && !$target.parent('a').addBack('a').length)) { - $('.o_edit_menu_popover').popover('hide'); - $('.o_edit_menu_popover').find('[data-toggle="tooltip"]').tooltip('hide'); + if ($target.parents('.o_edit_menu_popover').length && !$target.parent('a').addBack('a').length) { + ev.preventDefault(); } if ($target.is('a') && !$target.attr('data-oe-model') && !$target.find('> [data-oe-model]').length && $target.closest('#wrapwrap').length) { diff --git a/addons/website/static/tests/tours/edit_link_popover.js b/addons/website/static/tests/tours/edit_link_popover.js index 2960b01d19e..741444f6f21 100644 --- a/addons/website/static/tests/tours/edit_link_popover.js +++ b/addons/website/static/tests/tours/edit_link_popover.js @@ -6,10 +6,29 @@ const wTourUtils = require('website.tour_utils'); const FIRST_PARAGRAPH = '#wrap .s_text_image p:nth-child(2)'; -const clickFooter = { +const clickFooter = [{ content: "Save the link by clicking outside the URL input (not on a link element)", - trigger: 'footer h5', -}; + trigger: 'footer h5:first', +}, { + content: "Wait delayed click on footer", + trigger: '.o_we_customize_panel we-title:contains("Footer")', + run: function () {}, // it's a check +}]; + +const clickEditLink = [{ + content: "Click on Edit Link in Popover", + trigger: '.o_edit_menu_popover .o_we_edit_link', + // FIXME this run shouldnt be needed but click not working as real click + run: (actions) => { + actions.click(); + $('.o_edit_menu_popover').popover('hide'); + }, +}, { + content: "Ensure popover is closed", + trigger: 'html:not(:has(.o_edit_menu_popover))', // popover should be closed + run: function () {}, // it's a check + in_modal: false, +}]; tour.register('edit_link_popover', { test: true, @@ -29,11 +48,11 @@ tour.register('edit_link_popover', { trigger: "#toolbar #create-link", }, { - content: "Type the link URL", + content: "Type the link URL /contactus", trigger: '#o_link_dialog_url_input', run: 'text /contactus' }, - clickFooter, + ...clickFooter, { content: "Click on newly created link", trigger: `${FIRST_PARAGRAPH} a`, @@ -43,19 +62,21 @@ tour.register('edit_link_popover', { trigger: '.o_edit_menu_popover .o_we_url_link:contains("Contact Us")', // At this point preview is loaded run: function () {}, // it's a check }, + ...clickEditLink, { - content: "Click on Edit Link in Popover", - trigger: '.o_edit_menu_popover .o_we_edit_link', - }, - { - content: "Type the link URL", + content: "Type the link URL /", trigger: '#o_link_dialog_url_input', run: "text /" }, - clickFooter, + ...clickFooter, { content: "Click on link", trigger: `${FIRST_PARAGRAPH} a`, + // FIXME this run shouldnt be needed but click not working as real click + run: function (actions) { + actions.click(); + this.$anchor.popover('show'); + }, }, { content: "Popover should be shown with updated preview data", @@ -69,6 +90,15 @@ tour.register('edit_link_popover', { { content: "Link should be removed", trigger: `${FIRST_PARAGRAPH}:not(:has(a))`, + // run: function () {}, // it's a check + // FIXME this run shouldnt be needed but click not working as real click + run: (actions) => { + $('.o_edit_menu_popover').popover('hide'); + }, + }, + { + content: "Ensure popover is closed", + trigger: 'html:not(:has(.o_edit_menu_popover))', // popover should be closed run: function () {}, // it's a check }, // 2. Test links in navbar (website) @@ -81,10 +111,7 @@ tour.register('edit_link_popover', { trigger: '.o_edit_menu_popover .o_we_url_link:contains("Home")', run: function () {}, // it's a check }, - { - content: "Click on Edit Link in Popover", - trigger: '.o_edit_menu_popover .o_we_edit_link', - }, + ...clickEditLink, { content: "Change the URL", trigger: '#o_link_dialog_url_input', @@ -96,11 +123,16 @@ tour.register('edit_link_popover', { }, { content: "Click on the Home menu again", - trigger: '#top_menu a:contains("Home")', extra_trigger: '#top_menu a:contains("Home")[href="/contactus"]', // href should be changed + trigger: '#top_menu a:contains("Home")', + // FIXME this run shouldnt be needed but click not working as real click + run: function (actions) { + actions.click(); + this.$anchor.popover('show'); + }, }, { - content: "Popover should be shown with updated preview data", + content: "Popover should be shown with updated preview data (2)", trigger: '.o_edit_menu_popover .o_we_url_link:contains("Contact Us")', run: function () {}, // it's a check }, @@ -137,31 +169,36 @@ tour.register('edit_link_popover', { run: function () {}, // it's a check }, // 4. Popover should close when clicking non-link element + ...clickFooter, + // FIXME this step shouldnt be needed but click not working as real click { - content: "Ensure popover is closed", - trigger: 'footer h5', + content: "REMOVEME", + trigger: '.o_edit_menu_popover', + run: (actions) => { + $('.o_edit_menu_popover').popover('hide'); + }, }, - // 5. Double click shouldn't do anything + // 5. Double click should not open popover but should open toolbar link { content: "Double click on link", - trigger: 'html:not(:has(.o_edit_menu_popover))', // popover should be closed - run: function () { - const $footerHomeLink = $('footer a[href="/"]').first(); - + extra_trigger: 'html:not(:has(.o_edit_menu_popover))', // popover should be closed + trigger: 'footer a[href="/"]', + run: function (actions) { // Create range to simulate real double click, see pull request const range = document.createRange(); - range.selectNodeContents($footerHomeLink[0]); + range.selectNodeContents(this.$anchor[0]); const sel = window.getSelection(); sel.removeAllRanges(); sel.addRange(range); - - $footerHomeLink.click().dblclick(); + actions.click(); + actions.dblclick(); + // FIXME this step shouldnt be needed but click not working as real click + this.$anchor.popover('show'); }, }, { - content: "Ensure nothing happened on double click (except showing popover)", - extra_trigger: 'html:not(:has(#o_link_dialog_url_input))', - trigger: '.o_edit_menu_popover', + content: "Ensure popover is opened on double click, and so is right panel edit link", + trigger: 'html:has(#o_link_dialog_url_input):has(.o_edit_menu_popover)', run: function () {}, // it's a check }, ]);