From 7c6291570c4d8da17c8372376a554295315a0209 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Wed, 22 Aug 2018 21:07:07 -0400 Subject: [PATCH] Bug 1484483 - Part 1: Refactor the flex container listing out of the Flexbox Component into FlexContainerList. r=rcaliman - Rename FlexboxItem component to FlexContainerItem to make it less ambiguous. - Refactor the flex container listing out of the Flexbox Component into FlexContainerList. --HG-- rename : devtools/client/inspector/flexbox/components/FlexboxItem.js => devtools/client/inspector/flexbox/components/FlexContainerItem.js --- .../client/inspector/flexbox/actions/flexbox.js | 2 +- devtools/client/inspector/flexbox/actions/index.js | 2 +- .../{FlexboxItem.js => FlexContainerItem.js} | 76 +++++++++++----------- .../{Flexbox.js => FlexContainerList.js} | 55 +++++++--------- .../client/inspector/flexbox/components/Flexbox.js | 51 +++++++-------- .../client/inspector/flexbox/components/moz.build | 3 +- devtools/client/inspector/flexbox/flexbox.js | 1 - .../client/inspector/grids/components/GridItem.js | 18 +++-- .../inspector/grids/components/GridOutline.js | 49 +++++++------- .../browser_grids_color-in-rules-grid-toggle.js | 2 +- .../browser_grids_grid-list-color-picker-on-ESC.js | 2 +- ...owser_grids_grid-list-color-picker-on-RETURN.js | 2 +- .../test/browser_grids_persist-color-palette.js | 2 +- .../inspector/layout/components/LayoutApp.js | 6 ++ devtools/client/inspector/layout/layout.js | 11 +--- 15 files changed, 134 insertions(+), 148 deletions(-) rename devtools/client/inspector/flexbox/components/{FlexboxItem.js => FlexContainerItem.js} (72%) copy devtools/client/inspector/flexbox/components/{Flexbox.js => FlexContainerList.js} (56%) diff --git a/devtools/client/inspector/flexbox/actions/flexbox.js b/devtools/client/inspector/flexbox/actions/flexbox.js index aba403408dff..229ce7b4a498 100644 --- a/devtools/client/inspector/flexbox/actions/flexbox.js +++ b/devtools/client/inspector/flexbox/actions/flexbox.js @@ -33,7 +33,7 @@ module.exports = { }, /** - * Update the color used for the flexbox's highlighter. + * Updates the color used for the flexbox's highlighter. * * @param {String} color * The color to use for this nodeFront's flexbox highlighter. diff --git a/devtools/client/inspector/flexbox/actions/index.js b/devtools/client/inspector/flexbox/actions/index.js index d658815b2e4c..403e3d8d5d70 100644 --- a/devtools/client/inspector/flexbox/actions/index.js +++ b/devtools/client/inspector/flexbox/actions/index.js @@ -14,7 +14,7 @@ createEnum([ // Updates the flexbox state with the newly selected flexbox. "UPDATE_FLEXBOX", - // Update the color used for the overlay of a flexbox. + // Updates the color used for the overlay of a flexbox. "UPDATE_FLEXBOX_COLOR", // Updates the flexbox highlighted state. diff --git a/devtools/client/inspector/flexbox/components/FlexboxItem.js b/devtools/client/inspector/flexbox/components/FlexContainerItem.js similarity index 72% rename from devtools/client/inspector/flexbox/components/FlexboxItem.js rename to devtools/client/inspector/flexbox/components/FlexContainerItem.js index 8f292f1af28d..fd70abf12e03 100644 --- a/devtools/client/inspector/flexbox/components/FlexboxItem.js +++ b/devtools/client/inspector/flexbox/components/FlexContainerItem.js @@ -17,25 +17,25 @@ const ElementNode = REPS.ElementNode; const Types = require("../types"); -class FlexboxItem extends PureComponent { +class FlexContainerItem extends PureComponent { static get propTypes() { return { flexbox: PropTypes.shape(Types.flexbox).isRequired, getSwatchColorPickerTooltip: PropTypes.func.isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetFlexboxOverlayColor: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, onToggleFlexboxHighlighter: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } constructor(props) { super(props); - this.setFlexboxColor = this.setFlexboxColor.bind(this); this.onFlexboxCheckboxClick = this.onFlexboxCheckboxClick.bind(this); this.onFlexboxInspectIconClick = this.onFlexboxInspectIconClick.bind(this); + this.setFlexboxColor = this.setFlexboxColor.bind(this); } componentDidMount() { @@ -109,45 +109,45 @@ class FlexboxItem extends PureComponent { nodeFront, } = flexbox; - return dom.li( - {}, - dom.label( - {}, - dom.input( - { - type: "checkbox", - value: actorID, - checked: highlighted, - onChange: this.onFlexboxCheckboxClick, - } + return ( + dom.li({}, + dom.label({}, + dom.input( + { + type: "checkbox", + value: actorID, + checked: highlighted, + onChange: this.onFlexboxCheckboxClick, + } + ), + Rep( + { + defaultRep: ElementNode, + mode: MODE.TINY, + object: translateNodeFrontToGrip(nodeFront), + onDOMNodeMouseOut: () => onHideBoxModelHighlighter(), + onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront), + onInspectIconClick: () => this.onFlexboxInspectIconClick(nodeFront), + } + ) ), - Rep( + dom.div( { - defaultRep: ElementNode, - mode: MODE.TINY, - object: translateNodeFrontToGrip(nodeFront), - onDOMNodeMouseOut: () => onHideBoxModelHighlighter(), - onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront), - onInspectIconClick: () => this.onFlexboxInspectIconClick(nodeFront), + className: "flexbox-color-swatch", + style: { + backgroundColor: color, + }, + title: color, } - ) - ), - dom.div( - { - className: "flexbox-color-swatch", - style: { - backgroundColor: color, - }, - title: color, - } - ), - // The SwatchColorPicker relies on the nextSibling of the swatch element to apply - // the selected color. This is why we use a span in display: none for now. - // Ideally we should modify the SwatchColorPickerTooltip to bypass this requirement. - // See https://bugzilla.mozilla.org/show_bug.cgi?id=1341578 - dom.span({ className: "flexbox-color-value" }, color) + ), + // The SwatchColorPicker relies on the nextSibling of the swatch element to apply + // the selected color. This is why we use a span in display: none for now. + // Ideally we should modify the SwatchColorPickerTooltip to bypass this + // requirement. See https://bugzilla.mozilla.org/show_bug.cgi?id=1341578 + dom.span({ className: "flexbox-color-value" }, color) + ) ); } } -module.exports = FlexboxItem; +module.exports = FlexContainerItem; diff --git a/devtools/client/inspector/flexbox/components/Flexbox.js b/devtools/client/inspector/flexbox/components/FlexContainerList.js similarity index 56% copy from devtools/client/inspector/flexbox/components/Flexbox.js copy to devtools/client/inspector/flexbox/components/FlexContainerList.js index 47bdbc1e6837..c8bf0d580309 100644 --- a/devtools/client/inspector/flexbox/components/Flexbox.js +++ b/devtools/client/inspector/flexbox/components/FlexContainerList.js @@ -9,20 +9,20 @@ const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); const { getStr } = require("devtools/client/inspector/layout/utils/l10n"); -const FlexboxItem = createFactory(require("./FlexboxItem")); +const FlexContainerItem = createFactory(require("./FlexContainerItem")); const Types = require("../types"); -class Flexbox extends PureComponent { +class FlexContainerList extends PureComponent { static get propTypes() { return { flexbox: PropTypes.shape(Types.flexbox).isRequired, getSwatchColorPickerTooltip: PropTypes.func.isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetFlexboxOverlayColor: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, onToggleFlexboxHighlighter: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } @@ -30,42 +30,35 @@ class Flexbox extends PureComponent { const { flexbox, getSwatchColorPickerTooltip, - setSelectedNode, onHideBoxModelHighlighter, onSetFlexboxOverlayColor, onShowBoxModelHighlighterForNode, onToggleFlexboxHighlighter, + setSelectedNode, } = this.props; - return flexbox.actorID ? - dom.div({ id: "layout-flexbox-container" }, - dom.div({ className: "flexbox-content" }, - dom.div({ className: "flexbox-container" }, - dom.span({}, getStr("flexbox.overlayFlexbox")), - dom.ul( - { - id: "flexbox-list", - className: "devtools-monospace", - }, - FlexboxItem({ - key: flexbox.id, - flexbox, - getSwatchColorPickerTooltip, - setSelectedNode, - onHideBoxModelHighlighter, - onSetFlexboxOverlayColor, - onShowBoxModelHighlighterForNode, - onToggleFlexboxHighlighter, - }) - ) - ) + return ( + dom.div({ className: "flexbox-container" }, + dom.span({}, getStr("flexbox.overlayFlexbox")), + dom.ul( + { + id: "flexbox-list", + className: "devtools-monospace", + }, + FlexContainerItem({ + key: flexbox.id, + flexbox, + getSwatchColorPickerTooltip, + onHideBoxModelHighlighter, + onSetFlexboxOverlayColor, + onShowBoxModelHighlighterForNode, + onToggleFlexboxHighlighter, + setSelectedNode, + }) ) ) - : - dom.div({ className: "devtools-sidepanel-no-result" }, - getStr("flexbox.noFlexboxeOnThisPage") - ); + ); } } -module.exports = Flexbox; +module.exports = FlexContainerList; diff --git a/devtools/client/inspector/flexbox/components/Flexbox.js b/devtools/client/inspector/flexbox/components/Flexbox.js index 47bdbc1e6837..7213da9f22e7 100644 --- a/devtools/client/inspector/flexbox/components/Flexbox.js +++ b/devtools/client/inspector/flexbox/components/Flexbox.js @@ -9,7 +9,9 @@ const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); const { getStr } = require("devtools/client/inspector/layout/utils/l10n"); -const FlexboxItem = createFactory(require("./FlexboxItem")); +loader.lazyGetter(this, "FlexContainerList", function() { + return createFactory(require("./FlexContainerList")); +}); const Types = require("../types"); @@ -18,11 +20,11 @@ class Flexbox extends PureComponent { return { flexbox: PropTypes.shape(Types.flexbox).isRequired, getSwatchColorPickerTooltip: PropTypes.func.isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetFlexboxOverlayColor: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, onToggleFlexboxHighlighter: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } @@ -30,41 +32,36 @@ class Flexbox extends PureComponent { const { flexbox, getSwatchColorPickerTooltip, - setSelectedNode, onHideBoxModelHighlighter, onSetFlexboxOverlayColor, onShowBoxModelHighlighterForNode, onToggleFlexboxHighlighter, + setSelectedNode, } = this.props; - return flexbox.actorID ? + if (!flexbox.actorID) { + return ( + dom.div({ className: "devtools-sidepanel-no-result" }, + getStr("flexbox.noFlexboxeOnThisPage") + ) + ); + } + + return ( dom.div({ id: "layout-flexbox-container" }, dom.div({ className: "flexbox-content" }, - dom.div({ className: "flexbox-container" }, - dom.span({}, getStr("flexbox.overlayFlexbox")), - dom.ul( - { - id: "flexbox-list", - className: "devtools-monospace", - }, - FlexboxItem({ - key: flexbox.id, - flexbox, - getSwatchColorPickerTooltip, - setSelectedNode, - onHideBoxModelHighlighter, - onSetFlexboxOverlayColor, - onShowBoxModelHighlighterForNode, - onToggleFlexboxHighlighter, - }) - ) - ) + FlexContainerList({ + flexbox, + getSwatchColorPickerTooltip, + onHideBoxModelHighlighter, + onSetFlexboxOverlayColor, + onShowBoxModelHighlighterForNode, + onToggleFlexboxHighlighter, + setSelectedNode, + }) ) ) - : - dom.div({ className: "devtools-sidepanel-no-result" }, - getStr("flexbox.noFlexboxeOnThisPage") - ); + ); } } diff --git a/devtools/client/inspector/flexbox/components/moz.build b/devtools/client/inspector/flexbox/components/moz.build index 407f798b8c5d..9ca95765924b 100644 --- a/devtools/client/inspector/flexbox/components/moz.build +++ b/devtools/client/inspector/flexbox/components/moz.build @@ -6,5 +6,6 @@ DevToolsModules( 'Flexbox.js', - 'FlexboxItem.js', + 'FlexContainerItem.js', + 'FlexContainerList.js', ) diff --git a/devtools/client/inspector/flexbox/flexbox.js b/devtools/client/inspector/flexbox/flexbox.js index 16f38fe88681..55a12583c256 100644 --- a/devtools/client/inspector/flexbox/flexbox.js +++ b/devtools/client/inspector/flexbox/flexbox.js @@ -93,7 +93,6 @@ class FlexboxInspector { getComponentProps() { return { - getSwatchColorPickerTooltip: this.getSwatchColorPickerTooltip, onSetFlexboxOverlayColor: this.onSetFlexboxOverlayColor, onToggleFlexboxHighlighter: this.onToggleFlexboxHighlighter, }; diff --git a/devtools/client/inspector/grids/components/GridItem.js b/devtools/client/inspector/grids/components/GridItem.js index 0a1b6dba3ee9..f42c1f9117b6 100644 --- a/devtools/client/inspector/grids/components/GridItem.js +++ b/devtools/client/inspector/grids/components/GridItem.js @@ -109,16 +109,14 @@ class GridItem extends PureComponent { onChange: this.onGridCheckboxClick, } ), - Rep( - { - defaultRep: ElementNode, - mode: MODE.TINY, - object: translateNodeFrontToGrip(nodeFront), - onDOMNodeMouseOut: () => onHideBoxModelHighlighter(), - onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront), - onInspectIconClick: () => this.onGridInspectIconClick(nodeFront), - } - ) + Rep({ + defaultRep: ElementNode, + mode: MODE.TINY, + object: translateNodeFrontToGrip(nodeFront), + onDOMNodeMouseOut: () => onHideBoxModelHighlighter(), + onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront), + onInspectIconClick: () => this.onGridInspectIconClick(nodeFront), + }) ), dom.div( { diff --git a/devtools/client/inspector/grids/components/GridOutline.js b/devtools/client/inspector/grids/components/GridOutline.js index 1ee54ad7055b..d6ee6843d175 100644 --- a/devtools/client/inspector/grids/components/GridOutline.js +++ b/devtools/client/inspector/grids/components/GridOutline.js @@ -195,17 +195,16 @@ class GridOutline extends PureComponent { * Displays a message text "Cannot show outline for this grid". */ renderCannotShowOutlineText() { - return dom.div( - { - className: "grid-outline-text" - }, - dom.span( - { - className: "grid-outline-text-icon", - title: getStr("layout.cannotShowGridOutline.title") - } - ), - getStr("layout.cannotShowGridOutline") + return ( + dom.div({ className: "grid-outline-text" }, + dom.span( + { + className: "grid-outline-text-icon", + title: getStr("layout.cannotShowGridOutline.title") + } + ), + getStr("layout.cannotShowGridOutline") + ) ); } @@ -296,8 +295,8 @@ class GridOutline extends PureComponent { */ renderGridCell(id, gridFragmentIndex, x, y, rowNumber, columnNumber, color, gridAreaName, width, height) { - return dom.rect( - { + return ( + dom.rect({ key: `${id}-${rowNumber}-${columnNumber}`, className: "grid-outline-cell", "data-grid-area-name": gridAreaName, @@ -312,33 +311,35 @@ class GridOutline extends PureComponent { fill: "none", onMouseEnter: this.onHighlightCell, onMouseLeave: this.onHighlightCell, - } + }) ); } renderGridOutline(grid) { const { color } = grid; - return dom.g( - { - id: "grid-outline-group", - className: "grid-outline-group", - style: { color } - }, - this.renderGrid(grid) + return ( + dom.g( + { + id: "grid-outline-group", + className: "grid-outline-group", + style: { color } + }, + this.renderGrid(grid) + ) ); } renderGridOutlineBorder(borderWidth, borderHeight, color) { - return dom.rect( - { + return ( + dom.rect({ key: "border", className: "grid-outline-border", x: 0, y: 0, width: borderWidth, height: borderHeight - } + }) ); } diff --git a/devtools/client/inspector/grids/test/browser_grids_color-in-rules-grid-toggle.js b/devtools/client/inspector/grids/test/browser_grids_color-in-rules-grid-toggle.js index 2ce5b5a6f807..8609cc8fb4af 100644 --- a/devtools/client/inspector/grids/test/browser_grids_color-in-rules-grid-toggle.js +++ b/devtools/client/inspector/grids/test/browser_grids_color-in-rules-grid-toggle.js @@ -23,7 +23,7 @@ add_task(async function() { const { inspector, gridInspector, layoutView } = await openLayoutView(); const { document: doc } = gridInspector; const { store } = inspector; - const cPicker = layoutView.getSwatchColorPickerTooltip(); + const cPicker = layoutView.swatchColorPickerTooltip; const spectrum = cPicker.spectrum; const swatch = doc.querySelector(".grid-color-swatch"); diff --git a/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-ESC.js b/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-ESC.js index 922827b4f568..897576c63166 100644 --- a/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-ESC.js +++ b/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-ESC.js @@ -23,7 +23,7 @@ add_task(async function() { const { inspector, gridInspector, layoutView } = await openLayoutView(); const { document: doc } = gridInspector; const { store } = inspector; - const cPicker = layoutView.getSwatchColorPickerTooltip(); + const cPicker = layoutView.swatchColorPickerTooltip; const spectrum = cPicker.spectrum; const swatch = doc.querySelector(".grid-color-swatch"); diff --git a/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-RETURN.js b/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-RETURN.js index 495d166352a6..d73e4a70c360 100644 --- a/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-RETURN.js +++ b/devtools/client/inspector/grids/test/browser_grids_grid-list-color-picker-on-RETURN.js @@ -23,7 +23,7 @@ add_task(async function() { const { inspector, gridInspector, layoutView } = await openLayoutView(); const { document: doc } = gridInspector; const { store } = inspector; - const cPicker = layoutView.getSwatchColorPickerTooltip(); + const cPicker = layoutView.swatchColorPickerTooltip; const spectrum = cPicker.spectrum; const swatch = doc.querySelector(".grid-color-swatch"); diff --git a/devtools/client/inspector/grids/test/browser_grids_persist-color-palette.js b/devtools/client/inspector/grids/test/browser_grids_persist-color-palette.js index dd7941743fa9..15f905353dd3 100644 --- a/devtools/client/inspector/grids/test/browser_grids_persist-color-palette.js +++ b/devtools/client/inspector/grids/test/browser_grids_persist-color-palette.js @@ -23,7 +23,7 @@ add_task(async function() { const { inspector, gridInspector, layoutView, toolbox } = await openLayoutView(); const { document: doc } = gridInspector; const { store } = inspector; - const cPicker = layoutView.getSwatchColorPickerTooltip(); + const cPicker = layoutView.swatchColorPickerTooltip; const swatch = doc.querySelector(".grid-color-swatch"); info("Scrolling into view of the #grid color swatch."); diff --git a/devtools/client/inspector/layout/components/LayoutApp.js b/devtools/client/inspector/layout/components/LayoutApp.js index 8f391fc33844..25e8ac488185 100644 --- a/devtools/client/inspector/layout/components/LayoutApp.js +++ b/devtools/client/inspector/layout/components/LayoutApp.js @@ -16,6 +16,7 @@ const Flexbox = createFactory(require("devtools/client/inspector/flexbox/compone const Grid = createFactory(require("devtools/client/inspector/grids/components/Grid")); const BoxModelTypes = require("devtools/client/inspector/boxmodel/types"); +const FlexboxTypes = require("devtools/client/inspector/flexbox/types"); const GridTypes = require("devtools/client/inspector/grids/types"); const Accordion = createFactory(require("./Accordion")); @@ -35,6 +36,7 @@ class LayoutApp extends PureComponent { static get propTypes() { return { boxModel: PropTypes.shape(BoxModelTypes.boxModel).isRequired, + flexbox: PropTypes.shape(FlexboxTypes.flexbox).isRequired, getSwatchColorPickerTooltip: PropTypes.func.isRequired, grids: PropTypes.arrayOf(PropTypes.shape(GridTypes.grid)).isRequired, highlighterSettings: PropTypes.shape(GridTypes.highlighterSettings).isRequired, @@ -44,7 +46,11 @@ class LayoutApp extends PureComponent { onShowBoxModelEditor: PropTypes.func.isRequired, onShowBoxModelHighlighter: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, + onShowGridOutlineHighlight: PropTypes.func.isRequired, + onToggleFlexboxHighlighter: PropTypes.func.isRequired, + onToggleGeometryEditor: PropTypes.func.isRequired, onToggleGridHighlighter: PropTypes.func.isRequired, + onToggleShowGridAreas: PropTypes.func.isRequired, onToggleShowGridLineNumbers: PropTypes.func.isRequired, onToggleShowInfiniteLines: PropTypes.func.isRequired, setSelectedNode: PropTypes.func.isRequired, diff --git a/devtools/client/inspector/layout/layout.js b/devtools/client/inspector/layout/layout.js index 6aafa236780b..5bdb5c393bdf 100644 --- a/devtools/client/inspector/layout/layout.js +++ b/devtools/client/inspector/layout/layout.js @@ -23,8 +23,6 @@ class LayoutView { this.inspector = inspector; this.store = inspector.store; - this.getSwatchColorPickerTooltip = this.getSwatchColorPickerTooltip.bind(this); - this.init(); } @@ -62,7 +60,7 @@ class LayoutView { } = this.gridInspector.getComponentProps(); const layoutApp = LayoutApp({ - getSwatchColorPickerTooltip: this.getSwatchColorPickerTooltip, + getSwatchColorPickerTooltip: () => this.swatchColorPickerTooltip, onHideBoxModelHighlighter, onSetFlexboxOverlayColor, onSetGridOverlayColor, @@ -112,13 +110,6 @@ class LayoutView { this.store = null; } - /** - * Retrieve the shared SwatchColorPicker instance. - */ - getSwatchColorPickerTooltip() { - return this.swatchColorPickerTooltip; - } - get swatchColorPickerTooltip() { if (!this._swatchColorPickerTooltip) { this._swatchColorPickerTooltip = new SwatchColorPickerTooltip( -- 2.11.4.GIT