From 9905d32b7b428a1bbf4af7f14055c91e3c48b62a Mon Sep 17 00:00:00 2001 From: Ramon Wenger Date: Thu, 8 Feb 2024 22:30:34 +0100 Subject: [PATCH] Prevent nested ContentBlocks to also open a HighlightPopover Also add a Testcase for them Relates to MS-879 --- .../e2e/frontend/modules/highlights.cy.ts | 150 ++++++++++++------ client/src/components/ContentBlock.vue | 5 +- .../highlights/HighlightPopover.vue | 1 + 3 files changed, 104 insertions(+), 52 deletions(-) diff --git a/client/cypress/e2e/frontend/modules/highlights.cy.ts b/client/cypress/e2e/frontend/modules/highlights.cy.ts index 679cf5be..510d97d2 100644 --- a/client/cypress/e2e/frontend/modules/highlights.cy.ts +++ b/client/cypress/e2e/frontend/modules/highlights.cy.ts @@ -1,32 +1,54 @@ import { defaultModuleQueriesandMutations } from '../../../support/helpers'; +const getModule = (contents) => { + return { + chapters: [ + { + title: 'A Chapter', + contentBlocks: [ + { + title: 'A Content Block', + highlights: [], + id: contentBlockId, + contents, + }, + ], + }, + ], + }; +}; + +const defaultContents = [ + { + type: 'text_block', + id: 'some-content-component-id', + value: { + text: '

Dies ist ein Text mit ein paar Wörtern.

', + }, + }, +]; + +const contentListContents = [ + { + id: '16ea6f8a-99bc-4366-9f32-d7c3e7e39765', + type: 'content_list_item', + value: [ + { + id: '73f571e9-c049-41e7-8927-c4badbfb9b93', + type: 'text_block', + value: { + text: '

Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo

Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo Hallo

', + }, + }, + ], + }, +]; + const contentBlockId = window.btoa('ContentBlockNode:1'); const operations = { ...defaultModuleQueriesandMutations, ModuleDetailsQuery: { - module: { - chapters: [ - { - title: 'A Chapter', - contentBlocks: [ - { - title: 'A Content Block', - highlights: [], - id: contentBlockId, - contents: [ - { - type: 'text_block', - id: 'some-content-component-id', - value: { - text: '

Dies ist ein Text mit ein paar Wörtern.

', - }, - }, - ], - }, - ], - }, - ], - }, + module: getModule(defaultContents), }, AddHighlight: ({ input: { highlight } }) => { return { @@ -55,41 +77,44 @@ const selectText = (elm: Node, start: number, end: number) => { cy.window().then((win) => win.getSelection().addRange(range)); }; -describe('Highlights', () => { - beforeEach(() => { - cy.setup(); - cy.mockGraphqlOps({ - operations, - }); - }); - - it('visits a module and highlights a paragraph', () => { - cy.visit('/module/my-module'); - - /* - * Mark the text (programmatically, as Cypress has no API for this) - */ - cy.getByDataCy('text-block').then(($elm) => { - const textBlock = $elm[0]; - const paragraph = textBlock.querySelector('p'); - selectText(paragraph.childNodes[0], 2, 12); - }); +const markText = () => { + /* + * Mark the text (programmatically, as Cypress has no API for this) + */ + cy.getByDataCy('text-block').then(($elm) => { + const textBlock = $elm[0]; + const paragraph = textBlock.querySelector('p'); + selectText(paragraph.childNodes[0], 2, 12); /* * Also trigger the events manually */ cy.document().trigger('selectionchange'); - cy.getByDataCy('content-block-div').trigger('mouseup'); + cy.wrap(paragraph.parentNode).trigger('mouseup'); + }); + cy.getByDataCy('highlight-popover').should('be.visible'); + cy.getByDataCy('highlight-popover').should('have.length', 1); // there should only be one popover - cy.getByDataCy('highlight-popover').should('be.visible'); - cy.getByDataCy('highlight-popover').should('have.length', 1); // there should only be one popover + /* + * manually remove the selection, because cypress does not do that on a click, unlike a real browser + */ + cy.window().then((win) => { + const selection = win.getSelection(); + selection.removeAllRanges(); + }); +}; - /* - * manually remove the selection, because cypress does not do that on a click, unlike a real browser - */ - cy.window().then((win) => { - const selection = win.getSelection(); - selection.removeAllRanges(); +describe('Highlights', () => { + beforeEach(() => { + cy.setup(); + }); + + it('visits a module and highlights a paragraph', () => { + cy.mockGraphqlOps({ + operations, }); + cy.visit('/module/my-module'); + + markText(); // mark the text with yellow and check the text cy.getByDataCy('highlight-alpha').click(); @@ -99,6 +124,14 @@ describe('Highlights', () => { cy.getByDataCy('highlight-popover').should('have.length', 1); // there should only be one popover cy.getByDataCy('highlight-mark').should('have.length', 1); + // mark the text with yellow and check the text + cy.getByDataCy('highlight-beta').click(); + cy.getByDataCy('highlight-mark').should('contain', 'es ist ein'); + // + // we only want to have one of each element and not accidentally create multiple + cy.getByDataCy('highlight-popover').should('have.length', 1); // there should only be one popover + cy.getByDataCy('highlight-mark').should('have.length', 1); + // display the sidebar and popover and check them cy.getByDataCy('highlight-note').click(); cy.getByDataCy('highlight-popover').should('be.visible'); @@ -133,4 +166,19 @@ describe('Highlights', () => { cy.getByDataCy('highlight-sidebar').should('not.exist'); cy.getByDataCy('highlight-mark').should('not.exist'); }); + + it.only('visits a module with a ContentListItem and highlights some text', () => { + cy.mockGraphqlOps({ + operations: { + ...operations, + ModuleDetailsQuery: { + module: getModule(contentListContents), + }, + }, + }); + + cy.visit('/module/my-module'); + + markText(); + }); }); diff --git a/client/src/components/ContentBlock.vue b/client/src/components/ContentBlock.vue index f1d12b99..e9b508d3 100644 --- a/client/src/components/ContentBlock.vue +++ b/client/src/components/ContentBlock.vue @@ -329,6 +329,7 @@ const createHighlight = (highlight: AddHighlightArgument) => { }); }; +const isNested = computed(() => props.contentBlock.root); // if it's nested, a the parent has the root propert onMounted(() => { log.debug('onMounted ContentBlock called'); @@ -377,7 +378,9 @@ onMounted(() => { }; selectionHandler = getSelectionHandler(options); // add the listener from highlights - element.addEventListener('mouseup', selectionHandler); + if (!isNested.value) { + element.addEventListener('mouseup', selectionHandler); + } } }); diff --git a/client/src/components/highlights/HighlightPopover.vue b/client/src/components/highlights/HighlightPopover.vue index f5978b10..f32bb6b0 100644 --- a/client/src/components/highlights/HighlightPopover.vue +++ b/client/src/components/highlights/HighlightPopover.vue @@ -11,6 +11,7 @@ >