diff --git a/app/assets/javascripts/discourse/app/components/quote-button.js b/app/assets/javascripts/discourse/app/components/quote-button.js index 8948fe35633..139ee6237e2 100644 --- a/app/assets/javascripts/discourse/app/components/quote-button.js +++ b/app/assets/javascripts/discourse/app/components/quote-button.js @@ -46,6 +46,37 @@ function regexSafeStr(str) { return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } +function getRangeBoundaryRect(range, atEnd) { + // Don't mess with the original range as it results in weird behaviours + // where certain browsers will deselect the selection + const clone = range.cloneRange(range); + + // create a marker element containing a single invisible character + const markerElement = document.createElement("span"); + markerElement.appendChild(document.createTextNode("\ufeff")); + + // on mobile, collapse the range at the end of the selection + if (atEnd) { + clone.collapse(); + } + // insert the marker + clone.insertNode(markerElement); + + // retrieve the position of the marker + const boundaryRect = markerElement.getBoundingClientRect(); + boundaryRect.x += document.documentElement.scrollLeft; + boundaryRect.y += document.documentElement.scrollTop; + + // remove the marker + const parent = markerElement.parentNode; + parent.removeChild(markerElement); + + // merge back all text nodes so they don't get messed up + parent.normalize(); + + return boundaryRect; +} + export default Component.extend(KeyEnterEscape, { classNames: ["quote-button"], classNameBindings: [ @@ -184,42 +215,10 @@ export default Component.extend(KeyEnterEscape, { // on Desktop, shows the button at the beginning of the selection // on Mobile, shows the button at the end of the selection const isMobileDevice = this.site.isMobileDevice; - const { isIOS, isAndroid, isSafari, isOpera } = this.capabilities; + const { isIOS, isAndroid, isOpera } = this.capabilities; const showAtEnd = isMobileDevice || isIOS || isAndroid || isOpera; - // Don't mess with the original range as it results in weird behaviours - // where certain browsers will deselect the selection - const clone = firstRange.cloneRange(); - - // create a marker element containing a single invisible character - const markerElement = document.createElement("span"); - markerElement.appendChild(document.createTextNode("\ufeff")); - - // on mobile, collapse the range at the end of the selection - if (showAtEnd) { - clone.collapse(); - } - // insert the marker - clone.insertNode(markerElement); - - // retrieve the position of the marker - const $markerElement = $(markerElement); - const markerOffset = $markerElement.offset(); - const parentScrollLeft = $markerElement.parent().scrollLeft(); - const $quoteButton = $(this.element); - - // remove the marker - const parent = markerElement.parentNode; - parent.removeChild(markerElement); - // merge back all text nodes so they don't get messed up - parent.normalize(); - - // work around Safari that would sometimes lose the selection - if (isSafari) { - this._reselected = true; - selection.removeAllRanges(); - selection.addRange(clone); - } + const boundaryPosition = getRangeBoundaryRect(firstRange, showAtEnd); // change the position of the button schedule("afterRender", () => { @@ -227,37 +226,59 @@ export default Component.extend(KeyEnterEscape, { return; } - let top = markerOffset.top; - let left = markerOffset.left + Math.max(0, parentScrollLeft); - if (showAtEnd) { - top = top + 25; - left = Math.min( - left + 10, - window.innerWidth - this.element.clientWidth - 10 - ); - } else { - top = top - $quoteButton.outerHeight() - 5; - } + let top = 0; + let left = 0; + const pxFromSelection = 5; - if (isIOS) { + if (showAtEnd) { // The selection-handles on iOS have a hit area of ~50px radius // so we need to make sure our buttons are outside that radius + // Apply the same logic on all mobile devices for consistency + + top = boundaryPosition.bottom + pxFromSelection; + left = boundaryPosition.left; const safeRadius = 50; - const endHandlePosition = markerOffset; + const viewportEdgeMargin = 10; + + const endHandlePosition = boundaryPosition; const width = this.element.clientWidth; const possiblePositions = [ - { top, left }, - { top, left: endHandlePosition.left - width - safeRadius - 10 }, // move to left - { top, left: left + safeRadius }, // move to right - { top: top + safeRadius, left: endHandlePosition.left - width / 2 }, // centered below end handle + { + // move to left + top, + left: left - width - safeRadius, + }, + { + // move to right + top, + left: left + safeRadius, + }, + { + // centered below end handle + top: top + safeRadius, + left: left - width / 2, + }, ]; - let newPos; for (const pos of possiblePositions) { - if (pos.left < 0 || pos.left + width + 10 > window.innerWidth) { - continue; // Offscreen + // Ensure buttons are entirely within the viewport + pos.left = Math.max(viewportEdgeMargin, pos.left); + pos.left = Math.min( + window.innerWidth - viewportEdgeMargin - width, + pos.left + ); + + let clearOfStartHandle = true; + if (isAndroid) { + // On android, the start-selection handle extends below the line, so we need to avoid it as well: + const startHandlePosition = getRangeBoundaryRect(firstRange, false); + + clearOfStartHandle = + pos.top - startHandlePosition.bottom >= safeRadius || + pos.left + width <= startHandlePosition.left - safeRadius || + pos.left >= startHandlePosition.left + safeRadius; } const clearOfEndHandle = @@ -265,19 +286,20 @@ export default Component.extend(KeyEnterEscape, { pos.left + width <= endHandlePosition.left - safeRadius || pos.left >= endHandlePosition.left + safeRadius; - if (clearOfEndHandle) { - newPos = pos; + if (clearOfStartHandle && clearOfEndHandle) { + left = pos.left; + top = pos.top; break; } } - - if (newPos) { - left = newPos.left; - top = newPos.top; - } + } else { + // Desktop + top = + boundaryPosition.top - this.element.clientHeight - pxFromSelection; + left = boundaryPosition.left; } - $quoteButton.offset({ top, left }); + Object.assign(this.element.style, { top: `${top}px`, left: `${left}px` }); if (!this.animated) { // We only enable CSS transitions after the initial positioning