From a719bbaf9d1206b51f97212519892728ca1b9b0a Mon Sep 17 00:00:00 2001 From: Bianca Danforth Date: Thu, 15 Nov 2018 11:38:45 -0800 Subject: [PATCH 1/3] Fix #256: Change popup width in overflow menu With CustomizableUI.jsm (via the experimental `browser.customizableUI` API added in #157), it is possible to detect when the browserAction toolbar button is and is not in the Overflow menu. Because the overflow menu width varies by OS (for example, it is 377px wide on my Mac 10.13 and 425px wide on a Linux 14.04), this patch will detect whether or not the toolbar button is in the Overflow menu and adjust the width accordingly. This fix has the added benefit of allowing us to constrain the non-Overflow menu popup width (which is hopefully the vast majority use case) to the UX spec of 320px where previously we had set it to a max-width of 400px to try to accomodate Overflow menu issues. --- src/browser_action/index.css | 6 ++++-- src/browser_action/index.jsx | 6 ++++++ src/experiment_apis/customizableUI/api.js | 5 +++++ src/experiment_apis/customizableUI/schema.json | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/browser_action/index.css b/src/browser_action/index.css index 7a58ec9..c281d34 100644 --- a/src/browser_action/index.css +++ b/src/browser_action/index.css @@ -21,8 +21,10 @@ body { } #browser-action-app { - min-width: 320px; - max-width: 400px; /* Overflow menu popup width exceeds 320px and cannot be changed */ + width: 320px; +} + +#browser-action-app.overflow { width: 100%; } diff --git a/src/browser_action/index.jsx b/src/browser_action/index.jsx index 57dc807..6253e29 100644 --- a/src/browser_action/index.jsx +++ b/src/browser_action/index.jsx @@ -20,6 +20,12 @@ if (extractedProductJSON) { appProps.extractedProduct = JSON.parse(extractedProductJSON); } +(async function checkOverflow() { + if (await browser.customizableUI.isWidgetInOverflow('shopping-testpilot_mozilla_org-browser-action')) { + document.getElementById('browser-action-app').classList.add('overflow'); + } +}()); + ReactDOM.render( diff --git a/src/experiment_apis/customizableUI/api.js b/src/experiment_apis/customizableUI/api.js index 1af7a8b..50f6159 100644 --- a/src/experiment_apis/customizableUI/api.js +++ b/src/experiment_apis/customizableUI/api.js @@ -5,6 +5,7 @@ this.customizableUI = class extends ExtensionAPI { getAPI(context) { + const {Services} = ChromeUtils.import('resource://gre/modules/Services.jsm'); ChromeUtils.import('resource://gre/modules/ExtensionCommon.jsm'); const {EventManager} = ExtensionCommon; const {CustomizableUI} = ChromeUtils.import('resource:///modules/CustomizableUI.jsm', {}); @@ -25,6 +26,10 @@ this.customizableUI = class extends ExtensionAPI { }; }, ).api(), + async isWidgetInOverflow(widgetId) { + const browserWindow = Services.wm.getMostRecentWindow('navigator:browser'); + return CustomizableUI.getWidget(widgetId).forWindow(browserWindow).overflowed; + }, }, }; } diff --git a/src/experiment_apis/customizableUI/schema.json b/src/experiment_apis/customizableUI/schema.json index d177a40..6179b04 100644 --- a/src/experiment_apis/customizableUI/schema.json +++ b/src/experiment_apis/customizableUI/schema.json @@ -14,6 +14,21 @@ } ] } + ], + "functions": [ + { + "name": "isWidgetInOverflow", + "type": "function", + "description": "Returns a boolean for whether or not the provided widget in the provided window is in the overflow menu or not.", + "async": true, + "parameters": [ + { + "name": "widgetId", + "type": "string", + "description": "The id of the widget to check." + } + ] + } ] } ] From b6c508a16f425fda1565a7f5f6305c6d46cdf0dc Mon Sep 17 00:00:00 2001 From: Bianca Danforth Date: Fri, 16 Nov 2018 12:28:53 -0800 Subject: [PATCH 2/3] Detect overflow for explicit (un)pinning of toolbar button to overflow menu Detect toolbar button in fixed overflow menu: - Add a call to `getPlacementOfWidget` - This check covers the case when the toolbar button is explicitly pinned/unpinned to the Overflow menu by the user. Context: - The original patch only detected the toolbar button in the non-fixed overflow menu, which only covered the case when the toolbar button was moved into and out of the overflow menu by a window resize. - Per jaws, the fixed versus non-fixed Overflow menus live in two different areas of the browser chrome, where "the non-fixed overflow panel is the area that is basically an extension of the navigation-toolbar." - With both checks together, we cover both of the cases in which the toolbar button can be moved into and out of the Overflow menu. --- src/experiment_apis/customizableUI/api.js | 14 +++++++++----- src/experiment_apis/customizableUI/schema.json | 4 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/experiment_apis/customizableUI/api.js b/src/experiment_apis/customizableUI/api.js index 50f6159..d95270d 100644 --- a/src/experiment_apis/customizableUI/api.js +++ b/src/experiment_apis/customizableUI/api.js @@ -1,14 +1,14 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/* global ChromeUtils ExtensionAPI ExtensionCommon */ +/* global ChromeUtils ExtensionAPI */ this.customizableUI = class extends ExtensionAPI { getAPI(context) { - const {Services} = ChromeUtils.import('resource://gre/modules/Services.jsm'); - ChromeUtils.import('resource://gre/modules/ExtensionCommon.jsm'); + const {CustomizableUI} = ChromeUtils.import('resource:///modules/CustomizableUI.jsm'); + const {ExtensionCommon} = ChromeUtils.import('resource://gre/modules/ExtensionCommon.jsm'); const {EventManager} = ExtensionCommon; - const {CustomizableUI} = ChromeUtils.import('resource:///modules/CustomizableUI.jsm', {}); + const {Services} = ChromeUtils.import('resource://gre/modules/Services.jsm'); return { customizableUI: { onWidgetRemoved: new EventManager( @@ -27,8 +27,12 @@ this.customizableUI = class extends ExtensionAPI { }, ).api(), async isWidgetInOverflow(widgetId) { + const {area} = CustomizableUI.getPlacementOfWidget(widgetId); const browserWindow = Services.wm.getMostRecentWindow('navigator:browser'); - return CustomizableUI.getWidget(widgetId).forWindow(browserWindow).overflowed; + // First check is for the non-fixed overflow menu (e.g. widget moved by resizing window) + // Second is for fixed overflow menu (e.g. widget moved by (un)pinning button to overflow) + return (CustomizableUI.getWidget(widgetId).forWindow(browserWindow).overflowed + || area === 'widget-overflow-fixed-list'); }, }, }; diff --git a/src/experiment_apis/customizableUI/schema.json b/src/experiment_apis/customizableUI/schema.json index 6179b04..63a7c9f 100644 --- a/src/experiment_apis/customizableUI/schema.json +++ b/src/experiment_apis/customizableUI/schema.json @@ -5,7 +5,7 @@ { "name": "onWidgetRemoved", "type": "function", - "description": "Fired when a widget is removed from the browser chrome", + "description": "Fired when a widget is removed from the browser chrome.", "parameters": [ { "name": "widgetId", @@ -19,7 +19,7 @@ { "name": "isWidgetInOverflow", "type": "function", - "description": "Returns a boolean for whether or not the provided widget in the provided window is in the overflow menu or not.", + "description": "Determine whether or not a widget is in the overflow menu.", "async": true, "parameters": [ { From c60594f50e77f22ea7513d549436474fc05b6581 Mon Sep 17 00:00:00 2001 From: Bianca Danforth Date: Tue, 20 Nov 2018 17:04:13 -0800 Subject: [PATCH 3/3] Incorporate Osmose's feedback --- src/experiment_apis/customizableUI/api.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/experiment_apis/customizableUI/api.js b/src/experiment_apis/customizableUI/api.js index d95270d..0919694 100644 --- a/src/experiment_apis/customizableUI/api.js +++ b/src/experiment_apis/customizableUI/api.js @@ -31,7 +31,8 @@ this.customizableUI = class extends ExtensionAPI { const browserWindow = Services.wm.getMostRecentWindow('navigator:browser'); // First check is for the non-fixed overflow menu (e.g. widget moved by resizing window) // Second is for fixed overflow menu (e.g. widget moved by (un)pinning button to overflow) - return (CustomizableUI.getWidget(widgetId).forWindow(browserWindow).overflowed + return ( + CustomizableUI.getWidget(widgetId).forWindow(browserWindow).overflowed || area === 'widget-overflow-fixed-list'); }, },