Bug 1359759 - 2 - Clear React warnings in GridOutline and prevent re-renders; r=gl

Made the GridOutline component work with only 1 grid at a time.
It already did, but in a not so obvious way.

Removed the setState that happened during the render call to avoid
React warnings.

Cleaned up various data attribute to use the dataset property instead.

Removed the mouseover/out that controled the background color of the
highlighted cells. This now happens in CSS :hover, using currentColor.

Avoided React warnings related to missing "key" props.

Made changes to grid-inspector to avoid loops of re-renders when the
highlighter would highlight a cell on hover.
The component would wait for highlighter's events to dispatch store
actions. Instead, we dispatch them first, then when the events come, we
check if things have really changed! This way, the events will still have
effect if they come from the rule-view for instance, but not if they
come from the grid outline itself.

MozReview-Commit-ID: 6LK8B1P8iMU

--HG--
extra : rebase_source : 473ca1e1777d4e14f4f45a0b306b8153775cb1e6
This commit is contained in:
Patrick Brosset 2017-05-10 16:07:12 +02:00
Родитель 18ae59a2a5
Коммит db018b15bc
3 изменённых файлов: 129 добавлений и 75 удалений

Просмотреть файл

@ -41,7 +41,7 @@ module.exports = createClass({
getInitialState() {
return {
selectedGrids: [],
selectedGrid: null,
height: 0,
width: 0,
};
@ -55,15 +55,42 @@ module.exports = createClass({
},
componentWillReceiveProps({ grids }) {
if (this.state.selectedGrids.length < 2) {
this.setState({
height: 0,
width: 0,
});
let selectedGrid = grids.find(grid => grid.highlighted);
// Store the height of the grid container in the component state to prevent overflow
// issues. We want to store the width of the grid container as well so that the
// viewbox is only the calculated width of the grid outline.
let { width, height } = selectedGrid
? this.getTotalWidthAndHeight(selectedGrid)
: { width: 0, height: 0 };
this.setState({ height, width, selectedGrid });
},
/**
* Get the width and height of a given grid.
*
* @param {Object} grid
* A single grid container in the document.
* @return {Object} An object like { width, height }
*/
getTotalWidthAndHeight(grid) {
// TODO: We are drawing the first fragment since only one is currently being stored.
// In the future we will need to iterate over all fragments of a grid.
const { gridFragments } = grid;
const { rows, cols } = gridFragments[0];
let height = 0;
for (let i = 0; i < rows.lines.length - 1; i++) {
height += GRID_CELL_SCALE_FACTOR * (rows.tracks[i].breadth / 100);
}
this.setState({
selectedGrids: grids.filter(grid => grid.highlighted),
});
let width = 0;
for (let i = 0; i < cols.lines.length - 1; i++) {
width += GRID_CELL_SCALE_FACTOR * (cols.tracks[i].breadth / 100);
}
return { width, height };
},
/**
@ -114,14 +141,12 @@ module.exports = createClass({
onShowGridAreaHighlight,
onShowGridCellHighlight,
} = this.props;
const name = target.getAttribute("data-grid-area-name");
const id = target.getAttribute("data-grid-id");
const fragmentIndex = target.getAttribute("data-grid-fragment-index");
const color = target.getAttribute("stroke");
const rowNumber = target.getAttribute("data-grid-row");
const columnNumber = target.getAttribute("data-grid-column");
target.setAttribute("fill", color);
const name = target.dataset.gridAreaName;
const id = target.dataset.gridId;
const fragmentIndex = target.dataset.gridFragmentIndex;
const color = target.closest(".grid-cell-group").dataset.gridLineColor;
const rowNumber = target.dataset.gridRow;
const columnNumber = target.dataset.gridColumn;
if (name) {
onShowGridAreaHighlight(grids[id].nodeFront, name, color);
@ -140,11 +165,12 @@ module.exports = createClass({
* A single grid container in the document.
*/
renderGrid(grid) {
const { id, color, gridFragments } = grid;
// TODO: We are drawing the first fragment since only one is currently being stored.
// In the future we will need to iterate over all fragments of a grid.
let gridFragmentIndex = 0;
const { id, color, gridFragments } = grid;
const { rows, cols, areas } = gridFragments[gridFragmentIndex];
const numberOfColumns = cols.lines.length - 1;
const numberOfRows = rows.lines.length - 1;
const rectangles = [];
@ -152,19 +178,18 @@ module.exports = createClass({
let y = 1;
let width = 0;
let height = 0;
// The grid outline border height/width is the total height/width of grid cells drawn.
let totalHeight = 0;
let totalWidth = 0;
// Draw the cells contained within the grid outline border.
// Draw the cells contained within the grid outline border.
for (let rowNumber = 1; rowNumber <= numberOfRows; rowNumber++) {
height = GRID_CELL_SCALE_FACTOR * (rows.tracks[rowNumber - 1].breadth / 100);
for (let columnNumber = 1; columnNumber <= numberOfColumns; columnNumber++) {
width = GRID_CELL_SCALE_FACTOR * (cols.tracks[columnNumber - 1].breadth / 100);
const gridAreaName = this.getGridAreaName(columnNumber, rowNumber, areas);
const gridCell = this.renderGridCell(id, gridFragmentIndex, x, y,
rowNumber, columnNumber, color, gridAreaName, width, height);
rowNumber, columnNumber, color, gridAreaName,
width, height);
rectangles.push(gridCell);
x += width;
@ -172,26 +197,11 @@ module.exports = createClass({
x = 1;
y += height;
totalHeight += height;
}
// Find the total width of the grid container so we can draw the border for it
for (let columnNumber = 0; columnNumber < numberOfColumns; columnNumber++) {
totalWidth += GRID_CELL_SCALE_FACTOR * (cols.tracks[columnNumber].breadth / 100);
}
// Store the height of the grid container in the component state to prevent overflow
// issues. We want to store the width of the grid container as well so that the
// viewbox is only the calculated width of the grid outline.
if (totalHeight > this.state.height || totalWidth > this.state.width) {
this.setState({
height: totalHeight + 20,
width: totalWidth,
});
}
// Draw a rectangle that acts as the grid outline border.
const border = this.renderGridOutlineBorder(totalWidth, totalHeight, color);
const border = this.renderGridOutlineBorder(this.state.width, this.state.height,
color);
rectangles.unshift(border);
return rectangles;
@ -223,7 +233,8 @@ module.exports = createClass({
gridAreaName, width, height) {
return dom.rect(
{
className: "grid-outline-cell",
"key": `${id}-${rowNumber}-${columnNumber}`,
"className": "grid-outline-cell",
"data-grid-area-name": gridAreaName,
"data-grid-fragment-index": gridFragmentIndex,
"data-grid-id": id,
@ -234,31 +245,34 @@ module.exports = createClass({
width,
height,
fill: "none",
stroke: color,
onMouseOver: this.onMouseOverCell,
onMouseOut: this.onMouseLeaveCell,
}
);
},
renderGridOutline(grids) {
renderGridOutline(grid) {
let { color } = grid;
return dom.g(
{
className: "grid-cell-group",
"className": "grid-cell-group",
"data-grid-line-color": color,
"style": { color }
},
grids.map(grid => this.renderGrid(grid))
this.renderGrid(grid)
);
},
renderGridOutlineBorder(borderWidth, borderHeight, color) {
return dom.rect(
{
key: "border",
className: "grid-outline-border",
x: 1,
y: 1,
width: borderWidth,
height: borderHeight,
stroke: color,
height: borderHeight
}
);
},
@ -289,6 +303,7 @@ module.exports = createClass({
gridLineNumber, lineType) {
return dom.line(
{
key: `${id}-${lineType}-${gridLineNumber}`,
className: "grid-outline-line",
"data-grid-fragment-index": gridFragmentIndex,
"data-grid-id": id,
@ -306,12 +321,12 @@ module.exports = createClass({
);
},
renderGridLines(grids) {
renderGridLines(grid) {
return dom.g(
{
className: "grid-outline-lines",
},
grids.map(grid => this.renderLines(grid))
this.renderLines(grid)
);
},
@ -366,10 +381,8 @@ module.exports = createClass({
onShowGridAreaHighlight,
onShowGridCellHighlight,
} = this.props;
const id = target.getAttribute("data-grid-id");
const color = target.getAttribute("stroke");
target.setAttribute("fill", "none");
const id = target.dataset.gridId;
const color = target.closest(".grid-cell-group").dataset.gridLineColor;
onShowGridAreaHighlight(grids[id].nodeFront, null, color);
onShowGridCellHighlight(grids[id].nodeFront, color);
@ -382,29 +395,29 @@ module.exports = createClass({
onMouseLeaveLine({ target }) {
const { grids, onShowGridLineNamesHighlight } = this.props;
const fragmentIndex = target.getAttribute("data-grid-fragment-index");
const id = target.getAttribute("data-grid-id");
const color = target.getAttribute("data-grid-line-color");
const fragmentIndex = target.dataset.gridFragmentIndex;
const id = target.dataset.gridId;
const color = target.closest(".grid-cell-group").dataset.gridLineColor;
onShowGridLineNamesHighlight(grids[id].nodeFront, fragmentIndex, color);
},
onMouseOverLine({ target }) {
const { grids, onShowGridLineNamesHighlight } = this.props;
const fragmentIndex = target.getAttribute("data-grid-fragment-index");
const id = target.getAttribute("data-grid-id");
const lineNumber = target.getAttribute("data-grid-line-number");
const type = target.getAttribute("data-grid-line-type");
const color = target.getAttribute("data-grid-line-color");
const fragmentIndex = target.dataset.gridFragmentIndex;
const id = target.dataset.gridId;
const lineNumber = target.dataset.gridLineNumber;
const type = target.dataset.gridLineType;
const color = target.closest(".grid-cell-group").dataset.gridLineColor;
onShowGridLineNamesHighlight(grids[id].nodeFront, fragmentIndex, color,
lineNumber, type);
},
render() {
const { selectedGrids, height, width } = this.state;
const { selectedGrid, height, width } = this.state;
return selectedGrids.length ?
return selectedGrid ?
dom.svg(
{
className: "grid-outline",
@ -412,8 +425,8 @@ module.exports = createClass({
height: this.getHeight(),
viewBox: `${TRANSLATE_X} ${TRANSLATE_Y} ${width} ${height}`,
},
this.renderGridOutline(selectedGrids),
this.renderGridLines(selectedGrids)
this.renderGridOutline(selectedGrid),
this.renderGridLines(selectedGrid)
)
:
null;

Просмотреть файл

@ -222,6 +222,22 @@ GridInspector.prototype = {
dispatch(updateShowInfiniteLines(showInfinteLines));
},
showGridHighlighter(node, settings) {
this.lastHighlighterColor = settings.color;
this.lastHighlighterNode = node;
this.lastHighlighterState = true;
this.highlighters.showGridHighlighter(node, settings);
},
toggleGridHighlighter(node, settings) {
this.lastHighlighterColor = settings.color;
this.lastHighlighterNode = node;
this.lastHighlighterState = node !== this.highlighters.gridHighlighterShown;
this.highlighters.toggleGridHighlighter(node, settings);
},
/**
* Updates the grid panel by dispatching the new grid data. This is called when the
* layout view becomes visible or the view needs to be updated with new grid data.
@ -287,8 +303,22 @@ GridInspector.prototype = {
onHighlighterChange(event, nodeFront, options) {
let highlighted = event === "grid-highlighter-shown";
let { color } = options;
this.store.dispatch(updateGridHighlighted(nodeFront, highlighted));
this.store.dispatch(updateGridColor(nodeFront, color));
// Only tell the store that the highlighter changed if it did change.
// If we're still highlighting the same node, with the same color, no need to force
// a refresh.
if (this.lastHighlighterState !== highlighted ||
this.lastHighlighterNode !== nodeFront) {
this.store.dispatch(updateGridHighlighted(nodeFront, highlighted));
}
if (this.lastHighlighterColor !== color || this.lastHighlighterNode !== nodeFront) {
this.store.dispatch(updateGridColor(nodeFront, color));
}
this.lastHighlighterColor = null;
this.lastHighlighterNode = null;
this.lastHighlighterState = null;
},
/**
@ -325,7 +355,7 @@ GridInspector.prototype = {
for (let grid of grids) {
if (grid.nodeFront === node && grid.highlighted) {
let highlighterSettings = this.getGridHighlighterSettings(node);
this.highlighters.showGridHighlighter(node, highlighterSettings);
this.showGridHighlighter(node, highlighterSettings);
}
}
},
@ -349,7 +379,9 @@ GridInspector.prototype = {
highlighterSettings.showGridArea = gridAreaName;
highlighterSettings.color = color;
this.highlighters.showGridHighlighter(node, highlighterSettings);
this.showGridHighlighter(node, highlighterSettings);
this.store.dispatch(updateGridHighlighted(node, true));
},
/**
@ -377,7 +409,9 @@ GridInspector.prototype = {
highlighterSettings.showGridCell = { gridFragmentIndex, rowNumber, columnNumber };
highlighterSettings.color = color;
this.highlighters.showGridHighlighter(node, highlighterSettings);
this.showGridHighlighter(node, highlighterSettings);
this.store.dispatch(updateGridHighlighted(node, true));
},
/**
@ -406,10 +440,11 @@ GridInspector.prototype = {
lineNumber,
type
};
highlighterSettings.color = color;
this.highlighters.showGridHighlighter(node, highlighterSettings);
this.showGridHighlighter(node, highlighterSettings);
this.store.dispatch(updateGridHighlighted(node, true));
},
/**
@ -440,7 +475,10 @@ GridInspector.prototype = {
*/
onToggleGridHighlighter(node) {
let highlighterSettings = this.getGridHighlighterSettings(node);
this.highlighters.toggleGridHighlighter(node, highlighterSettings);
this.toggleGridHighlighter(node, highlighterSettings);
this.store.dispatch(updateGridHighlighted(node,
node !== this.highlighters.gridHighlighterShown));
},
/**
@ -461,7 +499,7 @@ GridInspector.prototype = {
for (let grid of grids) {
if (grid.highlighted) {
let highlighterSettings = this.getGridHighlighterSettings(grid.nodeFront);
this.highlighters.showGridHighlighter(grid.nodeFront, highlighterSettings);
this.showGridHighlighter(grid.nodeFront, highlighterSettings);
}
}
},
@ -484,7 +522,7 @@ GridInspector.prototype = {
for (let grid of grids) {
if (grid.highlighted) {
let highlighterSettings = this.getGridHighlighterSettings(grid.nodeFront);
this.highlighters.showGridHighlighter(grid.nodeFront, highlighterSettings);
this.showGridHighlighter(grid.nodeFront, highlighterSettings);
}
}
},

Просмотреть файл

@ -92,18 +92,21 @@
.grid-outline-border {
fill: none;
stroke: currentColor;
stroke-width: 0.75;
vector-effect: non-scaling-stroke;
}
.grid-outline-cell {
pointer-events: all;
stroke: currentColor;
stroke-dasharray: 0.5, 2;
vector-effect: non-scaling-stroke;
}
.grid-outline-cell:hover {
opacity: 0.45;
fill: currentColor;
}
.grid-outline-line {