diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts index 82b5181c0055..60b7d1f5391f 100644 --- a/browser/src/canvas/sections/CommentListSection.ts +++ b/browser/src/canvas/sections/CommentListSection.ts @@ -94,6 +94,9 @@ export class CommentSection extends app.definitions.canvasSectionObject { // To associate comment id with its index in commentList array. private idIndexMap: Map; + private annotationMinSize: number; + private annotationMaxSize: number; + constructor () { super(); @@ -119,6 +122,8 @@ export class CommentSection extends app.definitions.canvasSectionObject { this.sectionProperties.commentsAreListed = (this.sectionProperties.docLayer._docType === 'text' || this.sectionProperties.docLayer._docType === 'presentation' || this.sectionProperties.docLayer._docType === 'drawing') && !(window).mode.isMobile(); this.idIndexMap = new Map(); this.mobileCommentModalId = this.map.uiManager.generateModalId(this.mobileCommentId); + this.annotationMinSize = Number(getComputedStyle(document.documentElement).getPropertyValue('--annotation-min-size')); + this.annotationMaxSize = Number(getComputedStyle(document.documentElement).getPropertyValue('--annotation-max-size')); } public onInitialize (): void { @@ -1250,7 +1255,10 @@ export class CommentSection extends app.definitions.canvasSectionObject { this.sectionProperties.selectedComment.hide(); } - this.update(); + var previousAnimationState = this.disableLayoutAnimation; + this.disableLayoutAnimation = true; + this.update(true, false); + this.disableLayoutAnimation = previousAnimationState; } private showHideComments (): void { @@ -1798,10 +1806,10 @@ export class CommentSection extends app.definitions.canvasSectionObject { return this.disableLayoutAnimation ? 0 : undefined; // undefined means it will use default value } - private layoutUp (subList: any, actualPosition: Array, lastY: number): number { + private layoutUp (subList: any, actualPosition: Array, lastY: number, relayout: boolean = true): number { var height: number; for (var i = 0; i < subList.length; i++) { - height = subList[i].getCommentHeight(); + height = subList[i].getCommentHeight(relayout); lastY = subList[i].sectionProperties.data.anchorPix[1] + height < lastY ? subList[i].sectionProperties.data.anchorPix[1]: lastY - (height * app.dpiScale); (new L.PosAnimation()).run(subList[i].sectionProperties.container, {x: Math.round(actualPosition[0] / app.dpiScale), y: Math.round(lastY / app.dpiScale)}, this.getAnimationDuration()); if (!subList[i].isEdit()) @@ -1810,7 +1818,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { return lastY; } - private loopUp (startIndex: number, x: number, startY: number): number { + private loopUp (startIndex: number, x: number, startY: number, relayout: boolean = true): number { var tmpIdx = 0; var checkSelectedPart: boolean = this.mustCheckSelectedPart(); startY -= this.sectionProperties.marginY; @@ -1831,7 +1839,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { } while (tmpIdx > -1 && this.sectionProperties.commentList[tmpIdx].sectionProperties.data.parent === this.sectionProperties.commentList[tmpIdx + 1].sectionProperties.data.id); if (subList.length > 0) { - startY = this.layoutUp(subList, [x, subList[0].sectionProperties.data.anchorPix[1]], startY); + startY = this.layoutUp(subList, [x, subList[0].sectionProperties.data.anchorPix[1]], startY, relayout); i = i - subList.length; } else { i = tmpIdx; @@ -1841,7 +1849,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { return startY; } - private layoutDown (subList: any, actualPosition: Array, lastY: number): number { + private layoutDown (subList: any, actualPosition: Array, lastY: number, relayout: boolean = true): number { var selectedComment = subList[0] === this.sectionProperties.selectedComment; for (var i = 0; i < subList.length; i++) { lastY = subList[i].sectionProperties.data.anchorPix[1] > lastY ? subList[i].sectionProperties.data.anchorPix[1]: lastY; @@ -1849,6 +1857,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { var isRTL = document.documentElement.dir === 'rtl'; if (selectedComment) { + // FIXME: getBoundingClientRect is expensive and this is a hot path (called continuously during animations and scrolling) const posX = (this.sectionProperties.showSelectedBigger ? Math.round((document.getElementById('document-container').getBoundingClientRect().width - subList[i].sectionProperties.container.getBoundingClientRect().width)/2) : Math.round(actualPosition[0] / app.dpiScale) - this.sectionProperties.deflectionOfSelectedComment * (isRTL ? -1 : 1)); @@ -1857,14 +1866,14 @@ export class CommentSection extends app.definitions.canvasSectionObject { else (new L.PosAnimation()).run(subList[i].sectionProperties.container, {x: Math.round(actualPosition[0] / app.dpiScale), y: Math.round(lastY / app.dpiScale)}, this.getAnimationDuration()); - lastY += (subList[i].getCommentHeight() * app.dpiScale); + lastY += (subList[i].getCommentHeight(relayout) * app.dpiScale); if (!subList[i].isEdit()) subList[i].show(); } return lastY; } - private loopDown (startIndex: number, x: number, startY: number): number { + private loopDown (startIndex: number, x: number, startY: number, relayout: boolean = true): number { var tmpIdx = 0; var checkSelectedPart: boolean = this.mustCheckSelectedPart(); // Pass over all comments present @@ -1884,7 +1893,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { } while (tmpIdx < this.sectionProperties.commentList.length && this.sectionProperties.commentList[tmpIdx].sectionProperties.data.parent !== '0'); if (subList.length > 0) { - startY = this.layoutDown(subList, [x, subList[0].sectionProperties.data.anchorPix[1]], startY); + startY = this.layoutDown(subList, [x, subList[0].sectionProperties.data.anchorPix[1]], startY, relayout); i = i + subList.length; } else { i = tmpIdx; @@ -1898,6 +1907,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { if (this.sectionProperties.arrow) { document.getElementById('document-container').removeChild(this.sectionProperties.arrow); this.sectionProperties.arrow = null; + app.sectionContainer.requestReDraw(); } } @@ -1913,7 +1923,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { endPoint[1] = Math.floor(endPoint[1] / app.dpiScale); if (this.sectionProperties.arrow !== null) { - var line: SVGLineElement = (document.getElementById('comment-arrow-line')); + var line: SVGLineElement = (this.sectionProperties.arrow.firstElementChild); line.setAttribute('x1', String(startPoint[0])); line.setAttribute('y1', String(startPoint[1])); line.setAttribute('x2', String(endPoint[0])); @@ -1942,7 +1952,7 @@ export class CommentSection extends app.definitions.canvasSectionObject { } } - private doLayout (): void { + private doLayout (relayout: boolean = true): void { if ((window).mode.isMobile() || this.sectionProperties.docLayer._docType === 'spreadsheet') { if (this.sectionProperties.commentList.length > 0) this.orderCommentList(); @@ -1951,7 +1961,8 @@ export class CommentSection extends app.definitions.canvasSectionObject { if (this.sectionProperties.commentList.length > 0) { this.orderCommentList(); - this.resetCommentsSize(); + if (relayout) + this.resetCommentsSize(); var isRTL = document.documentElement.dir === 'rtl'; @@ -1983,21 +1994,20 @@ export class CommentSection extends app.definitions.canvasSectionObject { this.showArrow([tempCrd[0], tempCrd[1]], [posX, tempCrd[1]]); } } - else { + else this.hideArrow(); - app.sectionContainer.requestReDraw(); - } var lastY = 0; if (selectedIndex) { - this.loopUp(selectedIndex - 1, x, yOrigin); - lastY = this.loopDown(selectedIndex, x, yOrigin); + this.loopUp(selectedIndex - 1, x, yOrigin, relayout); + lastY = this.loopDown(selectedIndex, x, yOrigin, relayout); } else { - lastY = this.loopDown(0, x, topRight[1]); + lastY = this.loopDown(0, x, topRight[1], relayout); } } - this.resizeComments(); + if (relayout) + this.resizeComments(); lastY += this.containerObject.getDocumentTopLeft()[1]; if (lastY > app.file.size.pixels[1]) { @@ -2010,21 +2020,21 @@ export class CommentSection extends app.definitions.canvasSectionObject { this.disableLayoutAnimation = false; } - private layout (zoom: any = null): void { - if (zoom) - this.doLayout(); + private layout (immediate: any = null, relayout: boolean = true): void { + if (immediate) + this.doLayout(relayout); else if (!this.sectionProperties.layoutTimer) { - this.sectionProperties.layoutTimer = setTimeout(function() { + this.sectionProperties.layoutTimer = setTimeout(() => { delete this.sectionProperties.layoutTimer; - this.doLayout(); - }.bind(this), 10 /* ms */); + this.doLayout(relayout); + }, 10 /* ms */); } // else - avoid excessive re-layout } - private update (): void { - if (this.sectionProperties.docLayer._docType === 'text') + private update (immediate: boolean = false, relayout: boolean = true): void { + if (relayout && this.sectionProperties.docLayer._docType === 'text') this.updateThreadInfoIndicator(); - this.layout(); + this.layout(immediate, relayout); } private updateThreadInfoIndicator(): void { @@ -2140,15 +2150,11 @@ export class CommentSection extends app.definitions.canvasSectionObject { // reset theis size to default (100px text) private resetCommentsSize (): void { if (this.sectionProperties.docLayer._docType === 'text') { - const minMaxHeight = Number(getComputedStyle(document.documentElement).getPropertyValue('--annotation-min-size')); for (var i = 0; i < this.sectionProperties.commentList.length;i++) { - if (this.sectionProperties.commentList[i].sectionProperties.contentNode.style.display !== 'none') - this.sectionProperties.commentList[i].sectionProperties.contentNode.style.maxHeight = minMaxHeight + 'px'; - } - if (this.sectionProperties.selectedComment) { - if (this.sectionProperties.selectedComment.sectionProperties.contentNode.style.display !== 'none') { - const maxMaxHeight = Number(getComputedStyle(document.documentElement).getPropertyValue('--annotation-max-size')); - this.sectionProperties.selectedComment.sectionProperties.contentNode.style.maxHeight = maxMaxHeight + 'px'; + if (this.sectionProperties.commentList[i].sectionProperties.contentNode.style.display !== 'none') { + const maxHeight = (this.sectionProperties.commentList[i] === this.sectionProperties.selectedComment) ? + this.annotationMaxSize : this.annotationMinSize; + this.sectionProperties.commentList[i].sectionProperties.contentNode.style.maxHeight = maxHeight + 'px'; } } } diff --git a/browser/src/canvas/sections/CommentSection.ts b/browser/src/canvas/sections/CommentSection.ts index a4623f4bf07a..16224cf1d5a4 100644 --- a/browser/src/canvas/sections/CommentSection.ts +++ b/browser/src/canvas/sections/CommentSection.ts @@ -44,6 +44,10 @@ export class Comment extends CanvasSectionObject { map: any; pendingInit: boolean = true; + cachedCommentHeight: number | null = null; + cachedIsEdit: boolean = false; + hidden: boolean | null = null; + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types constructor (data: any, options: any, commentListSectionPointer: cool.CommentSection) { super(); @@ -321,7 +325,7 @@ export class Comment extends CanvasSectionObject { posY: this.sectionProperties.children[i].sectionProperties.container._leaflet_pos.y}); } childPositions.sort((a, b) => { return a.posY - b.posY; }); - let lastPosY = this.sectionProperties.container._leaflet_pos.y + this.getCommentHeight(); + let lastPosY = this.sectionProperties.container._leaflet_pos.y + this.getCommentHeight(false); let i = 0; for (; i < childPositions.length; i++) { if (this.sectionProperties.childLines[i] === undefined) { @@ -337,9 +341,10 @@ export class Comment extends CanvasSectionObject { lastPosY = childPositions[i].posY + 24; } if (i < this.sectionProperties.childLines.length) { - for (let j = i; j < this.sectionProperties.childLines.length; j++) + for (let j = i; j < this.sectionProperties.childLines.length; j++) { this.sectionProperties.childLinesNode.removeChild(this.sectionProperties.childLines[i]); - this.sectionProperties.childLines.splice(i); + this.sectionProperties.childLines.splice(i); + } } } @@ -782,6 +787,7 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.nodeModify.style.display = 'none'; this.sectionProperties.nodeReply.style.display = 'none'; this.sectionProperties.collapsedInfoNode.style.visibility = ''; + this.cachedIsEdit = false; } private showCalc() { @@ -789,6 +795,7 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.contentNode.style.display = ''; this.sectionProperties.nodeModify.style.display = 'none'; this.sectionProperties.nodeReply.style.display = 'none'; + this.cachedIsEdit = false; this.positionCalcComment(); if (!(window).mode.isMobile()) { @@ -821,6 +828,7 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.nodeModify.style.display = 'none'; this.sectionProperties.nodeReply.style.display = 'none'; this.sectionProperties.contentNode.style.display = ''; + this.cachedIsEdit = false; if (this.isSelected() || !this.isCollapsed) { this.sectionProperties.container.style.visibility = ''; } @@ -844,6 +852,9 @@ export class Comment extends CanvasSectionObject { public show(): void { this.doPendingInitializationInView(true /* force */); + + if (this.hidden === false && !this.isEdit()) return; + this.showMarker(); // On mobile, container shouldn't be 'document-container', but it is 'document-container' on initialization. So we hide the comment until comment wizard is opened. @@ -852,11 +863,16 @@ export class Comment extends CanvasSectionObject { if (cool.CommentSection.commentWasAutoAdded) return; - if (this.sectionProperties.docLayer._docType === 'text') + + // We don't cache the hidden state for spreadsheets. Only one comment can be + // visible and they're hidden when scrolling, so it's easier this way. + if (this.sectionProperties.docLayer._docType === 'text') { this.showWriter(); - else if (this.sectionProperties.docLayer._docType === 'presentation' || this.sectionProperties.docLayer._docType === 'drawing') + this.hidden = false; + } else if (this.sectionProperties.docLayer._docType === 'presentation' || this.sectionProperties.docLayer._docType === 'drawing') { this.showImpressDraw(); - else if (this.sectionProperties.docLayer._docType === 'spreadsheet') + this.hidden = false; + } else if (this.sectionProperties.docLayer._docType === 'spreadsheet') this.showCalc(); this.setLayoutClass(); @@ -868,12 +884,15 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.nodeReply.style.display = 'none'; this.sectionProperties.showSelectedCoordinate = false; L.DomUtil.removeClass(this.sectionProperties.container, 'cool-annotation-collapsed-show'); + this.cachedIsEdit = false; + this.hidden = true; } private hideCalc() { this.sectionProperties.container.style.visibility = 'hidden'; this.sectionProperties.nodeModify.style.display = 'none'; this.sectionProperties.nodeReply.style.display = 'none'; + this.cachedIsEdit = false; if (this.sectionProperties.commentListSection.sectionProperties.selectedComment === this) this.sectionProperties.commentListSection.sectionProperties.selectedComment = null; @@ -891,8 +910,10 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.nodeModify.style.display = 'none'; this.sectionProperties.nodeReply.style.display = 'none'; + this.cachedIsEdit = false; } L.DomUtil.removeClass(this.sectionProperties.container, 'cool-annotation-collapsed-show'); + this.hidden = true; } // check if this is "our" autosaved comment @@ -907,7 +928,7 @@ export class Comment extends CanvasSectionObject { } public hide (): void { - if (this.isEdit()) { + if (this.hidden === true || this.isEdit()) { return; } @@ -1091,6 +1112,7 @@ export class Comment extends CanvasSectionObject { return; } if (!this.sectionProperties.commentContainerRemoved) { + this.cachedIsEdit = false; $(this.sectionProperties.container).removeClass('annotation-active reply-annotation-container modify-annotation-container'); this.removeLastBRTag(this.sectionProperties.nodeModifyText); if (this.sectionProperties.contentText.origText !== this.sectionProperties.nodeModifyText.innerText || @@ -1128,6 +1150,8 @@ export class Comment extends CanvasSectionObject { } else { this.sectionProperties.nodeReply.style.display = 'none'; + if (!this.sectionProperties.nodeModify || this.sectionProperties.nodeModify.style.display === 'none') + this.cachedIsEdit = false; } } @@ -1154,6 +1178,7 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.contentNode.style.display = ''; this.sectionProperties.nodeModify.style.display = 'none'; this.sectionProperties.nodeReply.style.display = ''; + this.cachedIsEdit = true; return this; } @@ -1164,12 +1189,12 @@ export class Comment extends CanvasSectionObject { this.sectionProperties.nodeReply.style.display = 'none'; this.sectionProperties.container.style.visibility = ''; this.sectionProperties.contentNode.style.display = 'none'; + this.cachedIsEdit = true; return this; } public isEdit (): boolean { - return !this.pendingInit && ((this.sectionProperties.nodeModify && this.sectionProperties.nodeModify.style.display !== 'none') || - (this.sectionProperties.nodeReply && this.sectionProperties.nodeReply.style.display !== 'none')); + return this.cachedIsEdit; } public isModifying(): boolean { @@ -1535,8 +1560,13 @@ export class Comment extends CanvasSectionObject { return 1; // Comment list not fully initialized but we know we are not root } - public getCommentHeight(): number { - return this.sectionProperties.container.getBoundingClientRect().height - this.sectionProperties.childLinesNode.getBoundingClientRect().height; + public getCommentHeight(invalidateCache: boolean = true): number { + if (invalidateCache) + this.cachedCommentHeight = null; + if (this.cachedCommentHeight === null) + this.cachedCommentHeight = this.sectionProperties.container.getBoundingClientRect().height + - this.sectionProperties.childLinesNode.getBoundingClientRect().height; + return this.cachedCommentHeight; } public setCollapsed(): void { diff --git a/browser/src/dom/PosAnimation.js b/browser/src/dom/PosAnimation.js index cc258c30ae7a..b5ac8d1ec605 100644 --- a/browser/src/dom/PosAnimation.js +++ b/browser/src/dom/PosAnimation.js @@ -16,6 +16,17 @@ L.PosAnimation = L.Class.extend({ this.fire('start'); + // Skip some work if the duration is zero + if (duration <= 0) { + el.style[L.DomUtil.TRANSITION] = ''; + L.DomUtil.setPosition(el, newPos); + this._el._leaflet_pos = newPos; + this._inProgress = false; + this.fire('step').fire('end'); + this._el.dataset.transitioning = false; + return; + } + el.style[L.DomUtil.TRANSITION] = 'all ' + (isNaN(duration) ? 0.25 : 0) + 's cubic-bezier(0,0,' + (easeLinearity || 0.5) + ',1)';