diff --git a/devtools/client/inspector/fonts/actions/font-editor.js b/devtools/client/inspector/fonts/actions/font-editor.js index fbc23e717ac6..332506d5a634 100644 --- a/devtools/client/inspector/fonts/actions/font-editor.js +++ b/devtools/client/inspector/fonts/actions/font-editor.js @@ -5,20 +5,11 @@ "use strict"; const { - RESET_EDITOR, - UPDATE_AXIS_VALUE, UPDATE_EDITOR_VISIBILITY, - UPDATE_EDITOR_STATE, } = require("./index"); module.exports = { - resetFontEditor() { - return { - type: RESET_EDITOR, - }; - }, - toggleFontEditor(isVisible, selector = "") { return { type: UPDATE_EDITOR_VISIBILITY, @@ -27,20 +18,4 @@ module.exports = { }; }, - updateAxis(axis, value) { - return { - type: UPDATE_AXIS_VALUE, - axis, - value, - }; - }, - - updateFontEditor(fonts, properties = {}) { - return { - type: UPDATE_EDITOR_STATE, - fonts, - properties, - }; - }, - }; diff --git a/devtools/client/inspector/fonts/actions/index.js b/devtools/client/inspector/fonts/actions/index.js index a9770cb1e7d0..103830e90726 100644 --- a/devtools/client/inspector/fonts/actions/index.js +++ b/devtools/client/inspector/fonts/actions/index.js @@ -8,15 +8,6 @@ const { createEnum } = require("devtools/client/shared/enum"); createEnum([ - // Reset font editor to intial state. - "RESET_EDITOR", - - // Update the value of a variable font axis. - "UPDATE_AXIS_VALUE", - - // Update font editor with applicable fonts and user-defined CSS font properties. - "UPDATE_EDITOR_STATE", - // Toggle the visibiltiy of the font editor "UPDATE_EDITOR_VISIBILITY", diff --git a/devtools/client/inspector/fonts/components/FontAxis.js b/devtools/client/inspector/fonts/components/FontAxis.js deleted file mode 100644 index 0e3d59cb718c..000000000000 --- a/devtools/client/inspector/fonts/components/FontAxis.js +++ /dev/null @@ -1,78 +0,0 @@ -/* 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/. */ - -"use strict"; - -const { PureComponent } = require("devtools/client/shared/vendor/react"); -const dom = require("devtools/client/shared/vendor/react-dom-factories"); -const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); - -class FontAxis extends PureComponent { - static get propTypes() { - return { - defaultValue: PropTypes.oneOfType([ PropTypes.string, PropTypes.number ]), - label: PropTypes.string.isRequired, - min: PropTypes.oneOfType([ PropTypes.string, PropTypes.number ]), - max: PropTypes.oneOfType([ PropTypes.string, PropTypes.number ]), - name: PropTypes.string.isRequired, - onChange: PropTypes.func.isRequired, - showInput: PropTypes.bool, - step: PropTypes.oneOfType([ PropTypes.string, PropTypes.number ]), - value: PropTypes.string, - }; - } - - constructor(props) { - super(props); - - this.onChange = this.onChange.bind(this); - } - - onChange(e) { - this.props.onChange(this.props.name, e.target.value); - } - - render() { - const defaults = { - min: this.props.min, - max: this.props.max, - onChange: this.onChange, - step: this.props.step || 1, - value: this.props.value || this.props.defaultValue, - }; - - const range = dom.input( - { - ...defaults, - className: "font-axis-slider", - title: this.props.label, - type: "range", - } - ); - - const input = dom.input( - { - ...defaults, - className: "font-axis-input", - type: "number", - } - ); - - return dom.label( - { - className: "font-axis", - }, - dom.span( - { - className: "font-axis-label", - }, - this.props.label - ), - range, - this.props.showInput ? input : null - ); - } -} - -module.exports = FontAxis; diff --git a/devtools/client/inspector/fonts/components/FontEditor.js b/devtools/client/inspector/fonts/components/FontEditor.js index dcf9bb96d325..85b66fe91d99 100644 --- a/devtools/client/inspector/fonts/components/FontEditor.js +++ b/devtools/client/inspector/fonts/components/FontEditor.js @@ -4,96 +4,27 @@ "use strict"; -const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react"); +const { PureComponent } = require("devtools/client/shared/vendor/react"); const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); -const FontAxis = createFactory(require("./FontAxis")); - const Types = require("../types"); class FontEditor extends PureComponent { static get propTypes() { return { fontEditor: PropTypes.shape(Types.fontEditor).isRequired, - onAxisUpdate: PropTypes.func.isRequired, }; } - /** - * Naive implementation to get increment step for variable font axis that ensures - * a wide spectrum of precision based on range of values between min and max. - * - * @param {Number|String} min - * Minumum value for range. - * @param {Number|String} max - * Maximum value for range. - * @return {String} - * Step value used in range input for font axis. - */ - getAxisStep(min, max) { - let step = 1; - let delta = parseInt(max, 10) - parseInt(min, 10); - - if (delta <= 1) { - step = 0.001; - } else if (delta <= 10) { - step = 0.01; - } else if (delta <= 100) { - step = 0.1; - } - - return step.toString(); - } - - /** - * Get an array of FontAxis components for of the given variable font axis instances. - * If an axis is defined in the fontEditor store, use its value, else use the default. - * - * @param {Array} fontAxes - * Array of font axis instances - * @param {Object} editedAxes - * Object with axes and values edited by the user or predefined in the CSS - * declaration for font-variation-settings. - * @return {Array} - * Array of FontAxis components - */ - renderAxes(fontAxes = [], editedAxes) { - return fontAxes.map(axis => { - return FontAxis({ - min: axis.minValue, - max: axis.maxValue, - value: editedAxes[axis.tag] || axis.defaultValue, - step: this.getAxisStep(axis.minValue, axis.maxValue), - label: axis.name, - name: axis.tag, - onChange: this.props.onAxisUpdate, - showInput: true - }); - }); - } - - // Placeholder for non-variable font UI. - // Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1450695 - renderPlaceholder() { - return dom.div({}, "No fonts with variation axes apply to this element."); - } - render() { - const { fonts, axes } = this.props.fontEditor; - // For MVP use ony first font to show axes if available. - // Future implementations will allow switching between multiple fonts. - const fontAxes = (fonts[0] && fonts[0].variationAxes) ? fonts[0].variationAxes : null; + const { selector } = this.props.fontEditor; return dom.div( { className: "theme-sidebar inspector-tabpanel", - id: "sidebar-panel-fontinspector" - }, - fontAxes ? - this.renderAxes(fontAxes, axes) - : - this.renderPlaceholder() + id: "sidebar-panel-fonteditor" + }, `Placeholder for Font Editor panel for selector: ${selector}` ); } } diff --git a/devtools/client/inspector/fonts/components/FontsApp.js b/devtools/client/inspector/fonts/components/FontsApp.js index c46ff7dd7896..e768a0e79cac 100644 --- a/devtools/client/inspector/fonts/components/FontsApp.js +++ b/devtools/client/inspector/fonts/components/FontsApp.js @@ -20,7 +20,6 @@ class FontsApp extends PureComponent { fontData: PropTypes.shape(Types.fontData).isRequired, fontEditor: PropTypes.shape(Types.fontEditor).isRequired, fontOptions: PropTypes.shape(Types.fontOptions).isRequired, - onAxisUpdate: PropTypes.func.isRequired, onPreviewFonts: PropTypes.func.isRequired, }; } @@ -30,8 +29,7 @@ class FontsApp extends PureComponent { fontData, fontEditor, fontOptions, - onAxisUpdate, - onPreviewFonts, + onPreviewFonts } = this.props; return dom.div( @@ -42,7 +40,6 @@ class FontsApp extends PureComponent { fontEditor.isVisible ? FontEditor({ fontEditor, - onAxisUpdate, }) : FontOverview({ diff --git a/devtools/client/inspector/fonts/components/moz.build b/devtools/client/inspector/fonts/components/moz.build index 9e493db72df7..72275194e81f 100644 --- a/devtools/client/inspector/fonts/components/moz.build +++ b/devtools/client/inspector/fonts/components/moz.build @@ -6,7 +6,6 @@ DevToolsModules( 'Font.js', - 'FontAxis.js', 'FontEditor.js', 'FontList.js', 'FontOverview.js', diff --git a/devtools/client/inspector/fonts/fonts.js b/devtools/client/inspector/fonts/fonts.js index 927fffff9f31..f48cd4b6d3b6 100644 --- a/devtools/client/inspector/fonts/fonts.js +++ b/devtools/client/inspector/fonts/fonts.js @@ -10,7 +10,6 @@ const { gDevTools } = require("devtools/client/framework/devtools"); const { getColor } = require("devtools/client/shared/theme"); const { createFactory, createElement } = require("devtools/client/shared/vendor/react"); const { Provider } = require("devtools/client/shared/vendor/react-redux"); -const { throttle } = require("devtools/shared/throttle"); const FontsApp = createFactory(require("./components/FontsApp")); @@ -20,18 +19,9 @@ const INSPECTOR_L10N = const { updateFonts } = require("./actions/fonts"); const { updatePreviewText } = require("./actions/font-options"); -const { resetFontEditor, toggleFontEditor, updateAxis, updateFontEditor } = - require("./actions/font-editor"); +const { toggleFontEditor } = require("./actions/font-editor"); const FONT_EDITOR_ID = "fonteditor"; -const FONT_PROPERTIES = [ - "font-optical-sizing", - "font-size", - "font-stretch", - "font-style", - "font-variation-settings", - "font-weight", -]; class FontInspector { constructor(inspector, window) { @@ -42,15 +32,12 @@ class FontInspector { this.selectedRule = null; this.store = this.inspector.store; - this.syncChanges = throttle(this.syncChanges, 100, this); - this.onAxisUpdate = this.onAxisUpdate.bind(this); + this.update = this.update.bind(this); this.onNewNode = this.onNewNode.bind(this); this.onPreviewFonts = this.onPreviewFonts.bind(this); this.onRuleSelected = this.onRuleSelected.bind(this); this.onRuleUnselected = this.onRuleUnselected.bind(this); - this.onRuleUpdated = this.onRuleUpdated.bind(this); this.onThemeChanged = this.onThemeChanged.bind(this); - this.update = this.update.bind(this); this.init(); } @@ -62,7 +49,6 @@ class FontInspector { let fontsApp = FontsApp({ onPreviewFonts: this.onPreviewFonts, - onAxisUpdate: this.onAxisUpdate, }); let provider = createElement(Provider, { @@ -159,63 +145,6 @@ class FontInspector { this.inspector.sidebar.getCurrentTabID() === "fontinspector"; } - /** - * Live preview all CSS font property values from the fontEditor store on the page - * and sync the changes to the Rule view. - */ - applyChanges() { - const fontEditor = this.store.getState().fontEditor; - // Until registered axis values are supported as font property values, - // write all axes and their values to font-variation-settings. - // Bug 1449891: https://bugzilla.mozilla.org/show_bug.cgi?id=1449891 - const name = "font-variation-settings"; - const value = Object.keys(fontEditor.axes) - .map(tag => `"${tag}" ${fontEditor.axes[tag]}`) - .join(", "); - - let textProperty = this.selectedRule.textProps.filter(prop => prop.name === name)[0]; - if (!textProperty) { - textProperty = this.selectedRule.editor.addProperty(name, value, "", true); - } - - // Prevent reacting to changes we caused. - this.ruleView.off("property-value-updated", this.onRuleUpdated); - // Live preview font property changes on the page. - this.selectedRule.previewPropertyValue(textProperty, value, ""); - // Sync Rule view with changes reflected on the page (throttled). - this.syncChanges(textProperty, value); - } - - /** - * Sync the Rule view with the styles from the page. Called in a throttled way - * (see constructor) after property changes are applied directly to the CSS style rule - * on the page circumventing TextProperty.setValue() which triggers expensive DOM - * operations in TextPropertyEditor.update(). - * - * @param {TextProperty} textProperty - * Model of CSS declaration for a property in used in the rule view. - * @param {String} value - * Value of the CSS property that should be reflected in the rule view. - */ - syncChanges(textProperty, value) { - textProperty.updateValue(value); - this.ruleView.on("property-value-updated", this.onRuleUpdated); - } - - /** - * Handler for changes of font axis value. Updates the value in the store and previews - * the change on the page. - * - * @param {String} tag - * Tag name of the font axis. - * @param {String} value - * Value of the font axis. - */ - onAxisUpdate(tag, value) { - this.store.dispatch(updateAxis(tag, value)); - this.applyChanges(); - } - /** * Selection 'new-node' event handler. */ @@ -239,31 +168,20 @@ class FontInspector { * If selected for the font editor, hold a reference to the rule so we know where to * put property changes coming from the font editor and show the font editor panel. * - * @param {Object} eventData - * Data payload for the event. Contains: - * - {String} editorId - id of the editor for which the rule was selected - * - {Rule} rule - reference to rule that was selected + * @param {Object} eventData + * Data payload for the event. Contains: + * - {String} editorId - id of the editor for which the rule was selected + * - {Rule} rule - reference to rule that was selected */ - async onRuleSelected(eventData) { + onRuleSelected(eventData) { const { editorId, rule } = eventData; if (editorId === FONT_EDITOR_ID) { const selector = rule.matchedSelectors[0]; this.selectedRule = rule; - - await this.refreshFontEditor(); this.store.dispatch(toggleFontEditor(true, selector)); - this.ruleView.on("property-value-updated", this.onRuleUpdated); } } - /** - * Handler for "property-value-updated" event emitted from the rule view whenever a - * property value changes. - */ - async onRuleUpdated() { - await this.refreshFontEditor(); - } - /** * Handler for "ruleview-rule-unselected" event emitted from the rule view when a rule * was released from being selected for an editor. @@ -280,8 +198,6 @@ class FontInspector { if (editorId === FONT_EDITOR_ID && rule == this.selectedRule) { this.selectedRule = null; this.store.dispatch(toggleFontEditor(false)); - this.store.dispatch(resetFontEditor()); - this.ruleView.off("property-value-updated", this.onRuleUpdated); } } @@ -294,46 +210,6 @@ class FontInspector { } } - /** - * Update the state of the font editor with: - * - the fonts which apply to the current node; - * - the CSS font properties declared on the selected rule. - * - * This method is called during initial setup and as a result of any property - * values change in the Rule view. For the latter case, we do a deep compare between the - * font properties on the selected rule and the ones already store to decide if to - * update the font edtior to reflect a new external state. - */ - async refreshFontEditor() { - if (!this.selectedRule || !this.inspector || !this.store) { - return; - } - - const options = {}; - if (this.pageStyle.supportsFontVariations) { - options.includeVariations = true; - } - - const node = this.inspector.selection.nodeFront; - const fonts = await this.getFontsForNode(node, options); - // Collect any expected font properties and their values from the selected rule. - const properties = this.selectedRule.textProps.reduce((acc, prop) => { - if (FONT_PROPERTIES.includes(prop.name)) { - acc[prop.name] = prop.value; - } - - return acc; - }, {}); - - const fontEditor = this.store.getState().fontEditor; - - // Update the font editor state only if property values in rule differ from store. - // This can happen when a user makes manual edits to the values in the rule view. - if (JSON.stringify(properties) !== JSON.stringify(fontEditor.properties)) { - this.store.dispatch(updateFontEditor(fonts, properties)); - } - } - async update() { // Stop refreshing if the inspector or store is already destroyed. if (!this.inspector || !this.store) { diff --git a/devtools/client/inspector/fonts/reducers/font-editor.js b/devtools/client/inspector/fonts/reducers/font-editor.js index b93273d909b9..4d4ba48cffd8 100644 --- a/devtools/client/inspector/fonts/reducers/font-editor.js +++ b/devtools/client/inspector/fonts/reducers/font-editor.js @@ -5,58 +5,18 @@ "use strict"; const { - RESET_EDITOR, - UPDATE_AXIS_VALUE, - UPDATE_EDITOR_STATE, UPDATE_EDITOR_VISIBILITY, } = require("../actions/index"); const INITIAL_STATE = { - // Variable font axes. - axes: {}, - // Fonts applicable to selected element. - fonts: [], // Whether or not the font editor is visible. isVisible: false, - // CSS font properties defined on the selected rule. - properties: {}, - // Selector text of the selected rule where updated font properties will be written. + // Selector text of the rule where font properties will be written. selector: "", }; let reducers = { - [RESET_EDITOR](state) { - return { ...INITIAL_STATE }; - }, - - [UPDATE_AXIS_VALUE](state, { axis, value }) { - let newState = { ...state }; - newState.axes[axis] = value; - return newState; - }, - - [UPDATE_EDITOR_STATE](state, { fonts, properties }) { - let axes = {}; - - if (properties["font-variation-settings"]) { - // Parse font-variation-settings CSS declaration into an object - // with axis tags as keys and axis values as values. - axes = properties["font-variation-settings"] - .split(",") - .reduce((acc, pair) => { - // Tags are always in quotes. Split by quote and filter excessive whitespace. - pair = pair.split(/["']/).filter(part => part.trim() !== ""); - const tag = pair[0].trim(); - const value = pair[1].trim(); - acc[tag] = value; - return acc; - }, {}); - } - - return { ...state, axes, fonts, properties }; - }, - [UPDATE_EDITOR_VISIBILITY](state, { isVisible, selector }) { return { ...state, isVisible, selector }; }, diff --git a/devtools/client/inspector/fonts/types.js b/devtools/client/inspector/fonts/types.js index 5fd9268d5da9..5e9a3fa3748b 100644 --- a/devtools/client/inspector/fonts/types.js +++ b/devtools/client/inspector/fonts/types.js @@ -80,18 +80,12 @@ exports.fontOptions = { }; exports.fontEditor = { - // Variable font axes and their values - axes: PropTypes.object, - - // Fonts applicable to selected element - fonts: PropTypes.arrayOf(PropTypes.shape(font)), + // Font currently being edited + font: PropTypes.shape(font), // Whether or not the font editor is visible isVisible: PropTypes.bool, - // CSS font properties defined on the element - properties: PropTypes.object, - // Selector text of the rule where font properties will be written selector: PropTypes.string, }; diff --git a/devtools/client/inspector/rules/models/rule.js b/devtools/client/inspector/rules/models/rule.js index c45eeaf8e9e8..5af7e58ad079 100644 --- a/devtools/client/inspector/rules/models/rule.js +++ b/devtools/client/inspector/rules/models/rule.js @@ -252,7 +252,7 @@ Rule.prototype = { // relevant properties here. for (let index in modifications.changedDeclarations) { let newValue = modifications.changedDeclarations[index]; - this.textProps[index].updateValue(newValue); + this.textProps[index].noticeNewValue(newValue); } // Recompute and redisplay the computed properties. for (let prop of this.textProps) { diff --git a/devtools/client/inspector/rules/models/text-property.js b/devtools/client/inspector/rules/models/text-property.js index 0c6fc685f004..6d229d099942 100644 --- a/devtools/client/inspector/rules/models/text-property.js +++ b/devtools/client/inspector/rules/models/text-property.js @@ -128,12 +128,9 @@ TextProperty.prototype = { /** * Called when the property's value has been updated externally, and - * the property and editor should update to reflect that value. - * - * @param {String} value - * Property value + * the property and editor should update. */ - updateValue: function(value) { + noticeNewValue: function(value) { if (value !== this.value) { this.value = value; this.updateEditor(); diff --git a/devtools/client/themes/fonts.css b/devtools/client/themes/fonts.css index bb7d35c4c01f..b4ad25d1b990 100644 --- a/devtools/client/themes/fonts.css +++ b/devtools/client/themes/fonts.css @@ -8,10 +8,14 @@ flex-direction: column; width: 100%; height: 100%; - overflow: auto; +} + +#sidebar-panel-fonteditor { + padding: 1em; } #font-container { + overflow: auto; flex: auto; } @@ -109,39 +113,6 @@ vertical-align: middle; } -.font-axis { - display: flex; - flex-direction: row nowrap; - justify-content: space-between; - align-items: center; - padding: 5px 20px; -} - -.font-axis-input { - width: 60px; -} - -.font-axis-label { - width: 70px; -} - -.font-axis-slider { - flex: 1; -} - -.font-axis-slider::-moz-focus-outer { - border: 0; -} - -.font-axis-slider::-moz-range-thumb { - background: var(--grey-50); - border: 0; -} - -.font-axis-slider:focus::-moz-range-thumb { - background: var(--blue-55); -} - .font-origin { margin-top: .2em; color: var(--grey-50);