From 7b62c603509d69f6631c3d6d808407d226879b99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Ho=CC=88=C3=9Fl?= Date: Wed, 24 May 2017 22:50:25 +0200 Subject: [PATCH] Marks split list items with CSS-classes and hides the bullet points - fixes #3269 --- CHANGELOG | 2 + openslides/core/static/css/app.css | 28 +++- openslides/core/static/css/projector.css | 21 ++- openslides/core/static/js/core/base.js | 8 +- openslides/motions/static/js/motions/diff.js | 140 +++++++++++++++---- openslides/motions/static/js/motions/site.js | 5 +- tests/karma/motions/diff.service.test.js | 128 ++++++++++++++--- 7 files changed, 274 insertions(+), 58 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 10394a7bd..f495f2717 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -34,6 +34,8 @@ Motions: - Added config value for customize sorting of category list in pdf/docx export [#3329]. - Added config value for pagenumber alignment in PDF [#3327]. +- Bugfix: Several bugfixes regarding splitting list items in + change recommendations [#3288] Users: - User without permission to see users can now see agenda item speakers, diff --git a/openslides/core/static/css/app.css b/openslides/core/static/css/app.css index da95f4e76..dedd997c6 100644 --- a/openslides/core/static/css/app.css +++ b/openslides/core/static/css/app.css @@ -109,6 +109,18 @@ h4 { p { padding-bottom: 14px; } +p.os-split-after { + padding-bottom: 0; + margin-bottom: 0; +} +p.os-split-before { + padding-top: 0; + margin-top: 0; +} + +ul.os-split-after, ol.os-split-after { + margin-bottom: 0; +} a { text-decoration: none; @@ -607,6 +619,10 @@ img { /** Diff view */ +.motion-text-holder li.os-split-before { + list-style-type: none; +} + .change-recommendation-overview { background-color: #eee; border: solid 1px #ddd; @@ -657,19 +673,24 @@ img { border: solid 1px #eee; border-radius: 3px; margin-bottom: 0; - margin-top: -24px; padding-top: 0; padding-right: 155px; } .diff-box-transparent { margin-top: -24px; } +.motion-text-with-diffs li.os-split-before { + list-style-type: none; +} .motion-text-with-diffs .original-text { min-height: 30px; /* Spacer between .diff-box, in case .original-text is empty */ } -.motion-text-with-diffs .original-text ul:last-child { +.motion-text-with-diffs .original-text ul:last-child, .motion-text-with-diffs .original-text ol:last-child { padding-bottom: 16px; } +.motion-text-with-diffs .original-text ul.os-split-after:last-child, .motion-text-with-diffs .original-text ol.os-split-after:last-child { + padding-bottom: 0; +} .motion-text-with-diffs.line-numbers-inline .diff-box, .motion-text-with-diffs.line-numbers-none .diff-box { margin-right: -220px; } @@ -713,9 +734,6 @@ img { text-decoration: underline; } .motion-text-diff p { - padding-bottom: 0; - margin-top: 0; - margin-bottom: 0; } .motion-text-diff.line-numbers-outside .insert .os-line-number { display: none; diff --git a/openslides/core/static/css/projector.css b/openslides/core/static/css/projector.css index ef5d7ec04..b4fbf4531 100644 --- a/openslides/core/static/css/projector.css +++ b/openslides/core/static/css/projector.css @@ -538,7 +538,9 @@ tr.elected td { .diff-box { margin-left: 25px; padding-top: 0; - margin-top: -10px; +} +.motion-text-with-diffs li.os-split-before { + list-style-type: none; } .motion-text-with-diffs .original-text { min-height: 30px; // Spacer between .diff-box, in case .original-text is empty @@ -546,6 +548,16 @@ tr.elected td { .motion-text-with-diffs .original-text ul:last-child { padding-bottom: 16px; } +.motion-text-with-diffs .original-text ul.os-split-after:last-child { + padding-bottom: 0; +} +ol.os-split-after, ul.os-split-after { + margin-bottom: 0; + padding-bottom: 0; +} +p.os-split-after { + margin-bottom: 0; +} .motion-text-with-diffs.line-numbers-inline .diff-box, .motion-text-with-diffs.line-numbers-none .diff-box { margin-right: -220px; } @@ -560,8 +572,11 @@ tr.elected td { color: green; text-decoration: underline; } -.motion-text-diff p { - padding-bottom: 0; +.motion-text-diff p.os-split-before { + padding-top: 0; + margin-top: 0; +} +.motion-text-diff p.os-split-after { margin-top: 0; margin-bottom: 0; } diff --git a/openslides/core/static/js/core/base.js b/openslides/core/static/js/core/base.js index 56c93d570..152646efd 100644 --- a/openslides/core/static/js/core/base.js +++ b/openslides/core/static/js/core/base.js @@ -920,13 +920,13 @@ angular.module('OpenSlidesApp.core', [ allowedContent: 'h1 h2 h3 b i u strike sup sub strong em;' + 'blockquote p pre table' + - '(text-align-left,text-align-center,text-align-right,text-align-justify){text-align};' + + '(text-align-left,text-align-center,text-align-right,text-align-justify,os-split-before,os-split-after){text-align};' + 'a[!href];' + 'img[!src,alt]{width,height,float};' + 'tr th td caption;' + - 'li; ol[start]{list-style-type};' + - 'ul{list-style};' + - 'span[!*]{color,background-color}(os-line-number,line-number-*);' + + 'li(os-split-before,os-split-after); ol(os-split-before,os-split-after)[start]{list-style-type};' + + 'ul(os-split-before,os-split-after){list-style};' + + 'span[!*]{color,background-color}(os-split-before,os-split-after,os-line-number,line-number-*);' + 'br(os-line-break);', // there seems to be an error in CKeditor that parses spaces in extraPlugins as part of the plugin name. diff --git a/openslides/motions/static/js/motions/diff.js b/openslides/motions/static/js/motions/diff.js index 208b53725..920ec857a 100644 --- a/openslides/motions/static/js/motions/diff.js +++ b/openslides/motions/static/js/motions/diff.js @@ -308,6 +308,25 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi return fragment; }; + /** + * When a
  • with a os-split-before-class (set by extractRangeByLineNumbers) is edited when creating a + * change recommendation and is split again in CKEditor, the second list items also gets that class. + * This is not correct however, as the second one actually is a new list item. So we need to remove it again. + * + * @param {string} html + * @returns {string} + */ + this.removeDuplicateClassesInsertedByCkeditor = function(html) { + var fragment = this.htmlToFragment(html); + var items = fragment.querySelectorAll('li.os-split-before'); + for (var i = 0; i < items.length; i++) { + if (!this._isFirstNonemptyChild(items[i].parentNode, items[i])) { + this.removeCSSClass(items[i], 'os-split-before'); + } + } + return this._serializeDom(fragment, false); + }; + /** * Returns the HTML snippet between two given line numbers. * @@ -338,8 +357,18 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi * - followingHtml: The HTML after the selected area * - followingHtmlStartSnippet: A HTML snippet that opens all HTML tags necessary to render "followingHtml" * + * + * In some cases, the returned HTML tags receive additional CSS classes, providing information both for + * rendering it and for merging it again correctly. + * - os-split-*: These classes are set for all HTML Tags that have been split into two by this process, + * e.g. if the fromLine- or toLine-line-break was somewhere in the middle of this tag. + * If a tag is split, the first one receives "os-split-after", and the second one "os-split-before". + * For example, for the following string

    Line 1
    Line 2
    Line 3

    : + * - extracting line 1 to 2 results in

    Line 1

    + * - extracting line 2 to 3 results in

    Line 2

    + * - extracting line 3 to null/4 results in

    Line 3

    */ - this.extractRangeByLineNumbers = function(htmlIn, fromLine, toLine, debug) { + this.extractRangeByLineNumbers = function(htmlIn, fromLine, toLine) { if (typeof(htmlIn) !== 'string') { throw 'Invalid call - extractRangeByLineNumbers expects a string as first argument'; } @@ -384,27 +413,51 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi toChildTraceAbs.shift(); var followingHtml = this._serializePartialDomFromChild(fragment, toChildTraceAbs, false); - var currNode = fromLineNode.parentNode; + var currNode = fromLineNode, + isSplit = false; while (currNode.parentNode) { - previousHtmlEndSnippet += ''; + if (!this._isFirstNonemptyChild(currNode.parentNode, currNode)) { + isSplit = true; + } + if (isSplit) { + this.addCSSClass(currNode.parentNode, 'os-split-before'); + } + if (currNode.nodeName !== 'OS-LINEBREAK') { + previousHtmlEndSnippet += ''; + } currNode = currNode.parentNode; } - currNode = toLineNode.parentNode; + + currNode = toLineNode; + isSplit = false; while (currNode.parentNode) { - followingHtmlStartSnippet = this._serializeTag(currNode) + followingHtmlStartSnippet; + if (!this._isFirstNonemptyChild(currNode.parentNode, currNode)) { + isSplit = true; + } + if (isSplit) { + this.addCSSClass(currNode.parentNode, 'os-split-after'); + } + followingHtmlStartSnippet = this._serializeTag(currNode.parentNode) + followingHtmlStartSnippet; currNode = currNode.parentNode; } var found = false; + isSplit = false; for (var i = 0; i < fromChildTraceRel.length && !found; i++) { if (fromChildTraceRel[i].nodeName === 'OS-LINEBREAK') { found = true; } else { + if (!this._isFirstNonemptyChild(fromChildTraceRel[i], fromChildTraceRel[i + 1])) { + isSplit = true; + } if (fromChildTraceRel[i].nodeName === 'OL') { fakeOl = fromChildTraceRel[i].cloneNode(false); fakeOl.setAttribute('start', this._isWithinNthLIOfOL(fromChildTraceRel[i], fromLineNode)); innerContextStart += this._serializeTag(fakeOl); } else { + if (i < (fromChildTraceRel.length - 1) && isSplit) { + this.addCSSClass(fromChildTraceRel[i], 'os-split-before'); + } innerContextStart += this._serializeTag(fromChildTraceRel[i]); } } @@ -512,28 +565,28 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi var lastNode = nodes1[nodes1.length - 1], firstNode = nodes2[0]; - if (lastNode.nodeType == TEXT_NODE && firstNode.nodeType == TEXT_NODE) { + if (lastNode.nodeType === TEXT_NODE && firstNode.nodeType === TEXT_NODE) { var newTextNode = lastNode.ownerDocument.createTextNode(lastNode.nodeValue + firstNode.nodeValue); out.push(newTextNode); - } else if (lastNode.nodeName == firstNode.nodeName) { + } else if (lastNode.nodeName === firstNode.nodeName) { var newNode = lastNode.ownerDocument.createElement(lastNode.nodeName); for (i = 0; i < lastNode.attributes.length; i++) { var attr = lastNode.attributes[i]; newNode.setAttribute(attr.name, attr.value); } - // Remove #text nodes inside of List elements, as they are confusing + // Remove #text nodes inside of List elements (OL/UL), as they are confusing var lastChildren, firstChildren; - if (lastNode.nodeName == 'OL' || lastNode.nodeName == 'UL') { + if (lastNode.nodeName === 'OL' || lastNode.nodeName === 'UL') { lastChildren = []; firstChildren = []; for (i = 0; i < firstNode.childNodes.length; i++) { - if (firstNode.childNodes[i].nodeType == ELEMENT_NODE) { + if (firstNode.childNodes[i].nodeType === ELEMENT_NODE) { firstChildren.push(firstNode.childNodes[i]); } } for (i = 0; i < lastNode.childNodes.length; i++) { - if (lastNode.childNodes[i].nodeType == ELEMENT_NODE) { + if (lastNode.childNodes[i].nodeType === ELEMENT_NODE) { lastChildren.push(lastNode.childNodes[i]); } } @@ -546,12 +599,13 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi for (i = 0; i < children.length; i++) { newNode.appendChild(children[i]); } + out.push(newNode); } else { - if (lastNode.nodeName != 'TEMPLATE') { + if (lastNode.nodeName !== 'TEMPLATE') { out.push(lastNode); } - if (firstNode.nodeName != 'TEMPLATE') { + if (firstNode.nodeName !== 'TEMPLATE') { out.push(firstNode); } } @@ -570,9 +624,23 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi * @private */ this._normalizeHtmlForDiff = function (html) { - // Convert all HTML tags to uppercase, strip trailing whitespaces - html = html.replace(/<[^>]+>/g, function (tag) { - return tag.toUpperCase(); + // Convert all HTML tags to uppercase, but leave the values of attributes unchanged + html = html.replace(/<(\/?[a-z]*)( [^>]*)?>/ig, function (html, tag, attributes) { + var tagNormalized = tag.toUpperCase(); + if (attributes === undefined) { + attributes = ""; + } + attributes = attributes.replace(/( [^"'=]*)(= *((["'])(.*?)\4))?/gi, function (attr, attrName, attrRest, attrRest2, quot, attrValue) { + var attrNormalized = attrName.toUpperCase(); + if (attrRest !== undefined) { + if (attrNormalized === ' CLASS') { + attrValue = attrValue.split(' ').sort().join(' '); + } + attrNormalized += "=" + quot + attrValue + quot; + } + return attrNormalized; + }); + return "<" + tagNormalized + attributes + ">"; }); var entities = { @@ -649,7 +717,7 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi followingFragment = this.htmlToFragment(followingHtml), newFragment = this.htmlToFragment(newHTML); - if (data.html.length > 0 && data.html.substr(-1) == ' ') { + if (data.html.length > 0 && data.html.substr(-1) === ' ') { this._insertDanglingSpace(newFragment); } @@ -667,21 +735,46 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi el.parentNode.removeChild(el); } + var forgottenSplitClasses = mergedFragment.querySelectorAll(".os-split-before, .os-split-after"); + for (i = 0; i < forgottenSplitClasses.length; i++) { + this.removeCSSClass(forgottenSplitClasses[i], 'os-split-before'); + this.removeCSSClass(forgottenSplitClasses[i], 'os-split-after'); + } + return this._serializeDom(mergedFragment, true); }; this.addCSSClass = function (node, className) { - if (node.nodeType != ELEMENT_NODE) { + if (node.nodeType !== ELEMENT_NODE) { return; } var classes = node.getAttribute('class'); classes = (classes ? classes.split(' ') : []); - if (classes.indexOf(className) == -1) { + if (classes.indexOf(className) === -1) { classes.push(className); } node.setAttribute('class', classes.join(' ')); }; + this.removeCSSClass = function (node, className) { + if (node.nodeType !== ELEMENT_NODE) { + return; + } + var classes = node.getAttribute('class'), + newClasses = []; + classes = (classes ? classes.split(' ') : []); + for (var i = 0; i < classes.length; i++) { + if (classes[i] !== className) { + newClasses.push(classes[i]); + } + } + if (newClasses.length === 0) { + node.removeAttribute('class'); + } else { + node.setAttribute('class', newClasses.join(' ')); + } + }; + this.addDiffMarkup = function (originalHTML, newHTML, fromLine, toLine, diffFormatterCb) { var data = this.extractRangeByLineNumbers(originalHTML, fromLine, toLine), previousHtml = data.previousHtml + '' + data.previousHtmlEndSnippet, @@ -1151,19 +1244,14 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi // Remove tags that only delete line numbers diffUnnormalized = diffUnnormalized.replace( - /((
    )?]+OS-LINE-NUMBER[^>]+?>\s*<\/span>)<\/del>/gi, + /((
    )?]+os-line-number[^>]+?>\s*<\/span>)<\/del>/gi, function(found,tag) { return tag; } ); - // Lowercase line number markup diffUnnormalized = diffUnnormalized.replace( - /
    /gi, - '
    ' - ); - diffUnnormalized = diffUnnormalized.replace( - /]+OS-LINE-NUMBER[^>]+?>\s*<\/span>/gi, + /]+os-line-number[^>]+?>\s*<\/span>/gi, function(found) { return found.toLowerCase().replace(/> <\/span/gi, "> ' + - ' '; + ' '; }, noMarkup = function (no) { - return ' '; + return ' '; }; beforeEach(inject(function (_diffService_, _lineNumberingService_) { @@ -127,21 +127,52 @@ describe('linenumbering', function () { it('extracts a single line', function () { var diff = diffService.extractRangeByLineNumbers(baseHtml1, 1, 2); - expect(diff.html).toBe('

    Line 1 '); + expect(diff.html).toBe('

    Line 1 '); expect(diff.outerContextStart).toBe(''); expect(diff.outerContextEnd).toBe(''); }); it('extracts lines from nested UL/LI-structures', function () { var diff = diffService.extractRangeByLineNumbers(baseHtml1, 7, 9); - expect(diff.html).toBe('Line 7