Bug 1488660 - Fix devtools tab reordering in RTL locales;r=daisuke
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 19 Oct 2018 08:00:27 +0000
changeset 491211 ed6fb38d05c86fe07bf21d24c49e25ad90c29826
parent 491194 8ca56f27dc5864f2c8be0c6aa498cb24ebb7e65e
child 491212 3b0d0d3516bc47e317757feb7399e3e76330eb8c
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersdaisuke
bugs1488660
milestone65.0a1
Bug 1488660 - Fix devtools tab reordering in RTL locales;r=daisuke Differential Revision: https://phabricator.services.mozilla.com/D6910
devtools/client/framework/components/ToolboxTabs.js
devtools/client/framework/toolbox-tabs-order-manager.js
--- a/devtools/client/framework/components/ToolboxTabs.js
+++ b/devtools/client/framework/components/ToolboxTabs.js
@@ -55,18 +55,19 @@ class ToolboxTabs extends Component {
     // Map with tool Id and its width size. This lifecycle is out of React's
     // lifecycle. If a tool is registered, ToolboxTabs will add target tool id
     // to this map. ToolboxTabs will never remove tool id from this cache.
     this._cachedToolTabsWidthMap = new Map();
 
     this._resizeTimerId = null;
     this.resizeHandler = this.resizeHandler.bind(this);
 
+    const { toolbox, onTabsOrderUpdated, panelDefinitions } = props;
     this._tabsOrderManager =
-      new ToolboxTabsOrderManager(props.onTabsOrderUpdated, props.panelDefinitions);
+      new ToolboxTabsOrderManager(toolbox, onTabsOrderUpdated, panelDefinitions);
   }
 
   componentDidMount() {
     window.addEventListener("resize", this.resizeHandler);
     this.updateCachedToolTabsWidthMap();
     this.updateOverflowedTabs();
   }
 
--- a/devtools/client/framework/toolbox-tabs-order-manager.js
+++ b/devtools/client/framework/toolbox-tabs-order-manager.js
@@ -10,17 +10,18 @@ const Services = require("Services");
 const Telemetry = require("devtools/client/shared/telemetry");
 const TABS_REORDERED_SCALAR = "devtools.toolbox.tabs_reordered";
 const PREFERENCE_NAME = "devtools.toolbox.tabsOrder";
 
 /**
  * Manage the order of devtools tabs.
  */
 class ToolboxTabsOrderManager {
-  constructor(onOrderUpdated, panelDefinitions) {
+  constructor(toolbox, onOrderUpdated, panelDefinitions) {
+    this.toolbox = toolbox;
     this.onOrderUpdated = onOrderUpdated;
     this.currentPanelDefinitions = panelDefinitions || [];
 
     this.onMouseDown = this.onMouseDown.bind(this);
     this.onMouseMove = this.onMouseMove.bind(this);
     this.onMouseUp = this.onMouseUp.bind(this);
 
     Services.prefs.addObserver(PREFERENCE_NAME, this.onOrderUpdated);
@@ -48,16 +49,20 @@ class ToolboxTabsOrderManager {
     return !tabElement.previousSibling;
   }
 
   isLastTab(tabElement) {
     return !tabElement.nextSibling ||
            tabElement.nextSibling.id === "tools-chevron-menu-button";
   }
 
+  isRTL() {
+    return this.toolbox.direction === "rtl";
+  }
+
   async saveOrderPreference() {
     const tabs = [...this.toolboxTabsElement.querySelectorAll(".devtools-tab")];
     const tabIds = tabs.map(tab => tab.dataset.extensionId || tab.dataset.id);
     // Concat the overflowed tabs id since they are not contained in visible tabs.
     // The overflowed tabs cannot be reordered so we just append the id from current
     // panel definitions on their order.
     const overflowedTabIds =
       this.currentPanelDefinitions
@@ -99,49 +104,64 @@ class ToolboxTabsOrderManager {
     this.eventTarget.addEventListener("mousemove", this.onMouseMove);
     this.eventTarget.addEventListener("mouseup", this.onMouseUp);
 
     this.toolboxContainerElement.classList.add("tabs-reordering");
   }
 
   onMouseMove(e) {
     const diffPageX = e.pageX - this.previousPageX;
-    const dragTargetCenterX =
+    let dragTargetCenterX =
       this.dragTarget.offsetLeft + diffPageX + this.dragTarget.clientWidth / 2;
     let isDragTargetPreviousSibling = false;
 
-    for (const tabElement of this.toolboxTabsElement.querySelectorAll(".devtools-tab")) {
+    const tabElements = this.toolboxTabsElement.querySelectorAll(".devtools-tab");
+
+    // Calculate the minimum and maximum X-offset that can be valid for the drag target.
+    const firstElement = tabElements[0];
+    const firstElementCenterX = firstElement.offsetLeft + firstElement.clientWidth / 2;
+    const lastElement = tabElements[tabElements.length - 1];
+    const lastElementCenterX = lastElement.offsetLeft + lastElement.clientWidth / 2;
+    const max = Math.max(firstElementCenterX, lastElementCenterX);
+    const min = Math.min(firstElementCenterX, lastElementCenterX);
+
+    // Normalize the target center X so to remain between the first and last tab.
+    dragTargetCenterX = Math.min(max, dragTargetCenterX);
+    dragTargetCenterX = Math.max(min, dragTargetCenterX);
+
+    for (const tabElement of tabElements) {
       if (tabElement === this.dragTarget) {
         isDragTargetPreviousSibling = true;
         continue;
       }
 
+      // Is the dragTarget near the center of the other tab?
       const anotherCenterX = tabElement.offsetLeft + tabElement.clientWidth / 2;
-      const isReplaceable =
-        // Is the dragTarget near the center of the other tab?
-        Math.abs(dragTargetCenterX - anotherCenterX) < tabElement.clientWidth / 3 ||
-        // Has the dragTarget moved before the first tab
-        // (mouse moved too fast between two events)
-        (this.isFirstTab(tabElement) && dragTargetCenterX < anotherCenterX) ||
-        // Has the dragTarget moved after the last tab
-        // (mouse moved too fast between two events)
-        (this.isLastTab(tabElement) && anotherCenterX < dragTargetCenterX);
+      const distanceWithDragTarget = Math.abs(dragTargetCenterX - anotherCenterX);
+      const isReplaceable = distanceWithDragTarget < tabElement.clientWidth / 3;
 
       if (isReplaceable) {
         const replaceableElement =
           isDragTargetPreviousSibling ? tabElement.nextSibling : tabElement;
         this.insertBefore(replaceableElement);
         break;
       }
     }
 
     let distance = e.pageX - this.dragStartX;
 
-    if ((this.isFirstTab(this.dragTarget) && distance < 0) ||
-        (this.isLastTab(this.dragTarget) && distance > 0)) {
+    // To accomodate for RTL locales, we cannot rely on the first/last element of the
+    // NodeList. We cannot have negative distances for the leftmost tab, and we cannot
+    // have positive distances for the rightmost tab.
+    const isFirstTab = this.isFirstTab(this.dragTarget);
+    const isLastTab = this.isLastTab(this.dragTarget);
+    const isLeftmostTab = this.isRTL() ? isLastTab : isFirstTab;
+    const isRightmostTab = this.isRTL() ? isFirstTab : isLastTab;
+
+    if ((isLeftmostTab && distance < 0) || (isRightmostTab && distance > 0)) {
       // If the drag target is already edge of the tabs and the mouse will make the
       // element to move to same direction more, keep the position.
       distance = 0;
     }
 
     this.dragTarget.style.left = `${ distance }px`;
     this.previousPageX = e.pageX;
   }