Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Introduce BoxEdges typedef. #1816

Merged
merged 5 commits into from
Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/view/styles/border.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module engine/view/styles/border
*/

import { getShorthandValues, getTopRightBottomLeftValueReducer, getTopRightBottomLeftValues, isLength, isLineStyle } from './utils';
import { getShorthandValues, getBoxEdgesValueReducer, getBoxEdgesValues, isLength, isLineStyle } from './utils';

/**
* Adds a border CSS styles processing rules.
Expand Down Expand Up @@ -95,9 +95,9 @@ export function addBorderRules( stylesProcessor ) {
stylesProcessor.setExtractor( 'border-bottom-style', 'border.style.bottom' );
stylesProcessor.setExtractor( 'border-left-style', 'border.style.left' );

stylesProcessor.setReducer( 'border-color', getTopRightBottomLeftValueReducer( 'border-color' ) );
stylesProcessor.setReducer( 'border-style', getTopRightBottomLeftValueReducer( 'border-style' ) );
stylesProcessor.setReducer( 'border-width', getTopRightBottomLeftValueReducer( 'border-width' ) );
stylesProcessor.setReducer( 'border-color', getBoxEdgesValueReducer( 'border-color' ) );
stylesProcessor.setReducer( 'border-style', getBoxEdgesValueReducer( 'border-style' ) );
stylesProcessor.setReducer( 'border-width', getBoxEdgesValueReducer( 'border-width' ) );
stylesProcessor.setReducer( 'border-top', getBorderPositionReducer( 'top' ) );
stylesProcessor.setReducer( 'border-right', getBorderPositionReducer( 'right' ) );
stylesProcessor.setReducer( 'border-bottom', getBorderPositionReducer( 'bottom' ) );
Expand Down Expand Up @@ -134,9 +134,9 @@ function borderNormalizer( value ) {
return {
path: 'border',
value: {
color: getTopRightBottomLeftValues( color ),
style: getTopRightBottomLeftValues( style ),
width: getTopRightBottomLeftValues( width )
color: getBoxEdgesValues( color ),
style: getBoxEdgesValues( style ),
width: getBoxEdgesValues( width )
}
};
}
Expand Down Expand Up @@ -177,7 +177,7 @@ function getBorderPropertyNormalizer( propertyName ) {

function toBorderPropertyShorthand( value, property ) {
return {
[ property ]: getTopRightBottomLeftValues( value )
[ property ]: getBoxEdgesValues( value )
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/view/styles/margin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module engine/view/styles/margin
*/

import { getPositionShorthandNormalizer, getTopRightBottomLeftValueReducer } from './utils';
import { getPositionShorthandNormalizer, getBoxEdgesValueReducer } from './utils';

/**
* Adds a margin CSS styles processing rules.
Expand Down Expand Up @@ -35,7 +35,7 @@ export function addMarginRules( stylesProcessor ) {
stylesProcessor.setNormalizer( 'margin-bottom', value => ( { path: 'margin.bottom', value } ) );
stylesProcessor.setNormalizer( 'margin-left', value => ( { path: 'margin.left', value } ) );

stylesProcessor.setReducer( 'margin', getTopRightBottomLeftValueReducer( 'margin' ) );
stylesProcessor.setReducer( 'margin', getBoxEdgesValueReducer( 'margin' ) );

stylesProcessor.setStyleRelation( 'margin', [ 'margin-top', 'margin-right', 'margin-bottom', 'margin-left' ] );
}
4 changes: 2 additions & 2 deletions src/view/styles/padding.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module engine/view/styles/padding
*/

import { getPositionShorthandNormalizer, getTopRightBottomLeftValueReducer } from './utils';
import { getPositionShorthandNormalizer, getBoxEdgesValueReducer } from './utils';

/**
* Adds a margin CSS styles processing rules.
Expand All @@ -34,7 +34,7 @@ export function addPaddingRules( stylesProcessor ) {
stylesProcessor.setNormalizer( 'padding-bottom', value => ( { path: 'padding.bottom', value } ) );
stylesProcessor.setNormalizer( 'padding-left', value => ( { path: 'padding.left', value } ) );

stylesProcessor.setReducer( 'padding', getTopRightBottomLeftValueReducer( 'padding' ) );
stylesProcessor.setReducer( 'padding', getBoxEdgesValueReducer( 'padding' ) );

stylesProcessor.setStyleRelation( 'padding', [ 'padding-top', 'padding-right', 'padding-bottom', 'padding-left' ] );
}
16 changes: 8 additions & 8 deletions src/view/styles/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function isURL( string ) {
return urlRegExp.test( string );
}

export function getTopRightBottomLeftValues( value = '' ) {
export function getBoxEdgesValues( value = '' ) {
if ( value === '' ) {
return { top: undefined, right: undefined, bottom: undefined, left: undefined };
}
Expand All @@ -110,12 +110,12 @@ export function getTopRightBottomLeftValues( value = '' ) {
* Default reducer for CSS properties that concerns edges of a box
* [shorthand](https://developer.mozilla.org/en-US/docs/Web/CSS/Shorthand_properties) notations:
*
* stylesProcessor.setReducer( 'padding', getTopRightBottomLeftValueReducer( 'padding' ) );
* stylesProcessor.setReducer( 'padding', getBoxEdgesValueReducer( 'padding' ) );
*
* @param {String} styleShorthand
* @returns {Function}
*/
export function getTopRightBottomLeftValueReducer( styleShorthand ) {
export function getBoxEdgesValueReducer( styleShorthand ) {
return value => {
const { top, right, bottom, left } = value;

Expand All @@ -138,7 +138,7 @@ export function getTopRightBottomLeftValueReducer( styleShorthand ) {
reduced.push( [ styleShorthand + '-left', left ] );
}
} else {
reduced.push( [ styleShorthand, getTopRightBottomLeftShorthandValue( value ) ] );
reduced.push( [ styleShorthand, getBoxEdgesShorthandValue( value ) ] );
}

return reduced;
Expand All @@ -148,13 +148,13 @@ export function getTopRightBottomLeftValueReducer( styleShorthand ) {
/**
* Returns a proper 1-to-4 value of a CSS [shorthand](https://developer.mozilla.org/en-US/docs/Web/CSS/Shorthand_properties) notation.
*
* getTopRightBottomLeftShorthandValue( { top: '1px', right: '1px', bottom: '2px', left: '1px' } );
* getBoxEdgesShorthandValue( { top: '1px', right: '1px', bottom: '2px', left: '1px' } );
* // will return '1px 1px 2px'
*
* @param {String} styleShorthand
* @param {module:engine/view/stylesmap~BoxEdges} styleShorthand
* @returns {Function}
*/
export function getTopRightBottomLeftShorthandValue( { top, right, bottom, left } ) {
export function getBoxEdgesShorthandValue( { top, right, bottom, left } ) {
const out = [];

if ( left !== right ) {
Expand Down Expand Up @@ -182,7 +182,7 @@ export function getPositionShorthandNormalizer( shorthand ) {
return value => {
return {
path: shorthand,
value: getTopRightBottomLeftValues( value )
value: getBoxEdgesValues( value )
};
};
}
Expand Down
18 changes: 18 additions & 0 deletions src/view/stylesmap.js
Original file line number Diff line number Diff line change
Expand Up @@ -885,3 +885,21 @@ function appendStyleValue( stylesObject, nameOrPath, valueOrObject ) {
*
* @typedef {Array.<String, String>} module:engine/view/stylesmap~PropertyDescriptor
*/

/**
* An object describing box edges values.
*
* const margin = {
* top: '1px',
* right: '3px',
* bottom: '3px',
* left: '7px'
* }
*
* @typedef {Object} module:engine/view/stylesmap~BoxEdges
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should rather be box sides instead. Check out how w3 uses both terms https://www.w3.org/TR/CSS2/box.html (search for both):

These are for edges:

The perimeter of each of the four areas (content, padding, border, and margin) is called an "edge", so each box has four edges:

  • content edge or inner edge
  • padding edge
  • border edge
    ...

image

These are for sides (and IMO this is what we want to say):

The 'padding' shorthand property sets the padding for all four sides while the other padding properties only set their respective side.

If there is only one component value, it applies to all sides. If there are two values, the top and bottom paddings are set to the first value and the right and left paddings are set to the second.

The 'border-color' property can have from one to four component values, and the values are set on the different sides as for 'border-width'.

Summary

So edges are more like contours around the box (containing or not containing the padding, border, etc.) and the box has always 4 sides (top, left, bottom, right).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've looked at MDN docs and saw the edges used mostly. But your explanaition & wording is better. I'll fix that :)

*
* @property {String} top Top edge value.
* @property {String} right Right edge value.
* @property {String} bottom Bottom edge value.
* @property {String} left Left edge value.
*/
28 changes: 14 additions & 14 deletions tests/view/styles/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

import {
getShorthandValues,
getTopRightBottomLeftShorthandValue,
getTopRightBottomLeftValues,
getBoxEdgesShorthandValue,
getBoxEdgesValues,
isColor,
isLength,
isLineStyle
Expand Down Expand Up @@ -69,29 +69,29 @@ describe( 'Styles utils', () => {
} );
} );

describe( 'getTopRightBottomLeftShorthandValue()', () => {
describe( 'getBoxEdgesShorthandValue()', () => {
it( 'should output one value for same values', () => {
expect( getTopRightBottomLeftShorthandValue( { top: 'foo', right: 'foo', bottom: 'foo', left: 'foo' } ) ).to.equal( 'foo' );
expect( getBoxEdgesShorthandValue( { top: 'foo', right: 'foo', bottom: 'foo', left: 'foo' } ) ).to.equal( 'foo' );
} );

it( 'should output two value for top == bottom and right == left', () => {
expect( getTopRightBottomLeftShorthandValue( { top: 'foo', right: 'bar', bottom: 'foo', left: 'bar' } ) ).to.equal( 'foo bar' );
expect( getBoxEdgesShorthandValue( { top: 'foo', right: 'bar', bottom: 'foo', left: 'bar' } ) ).to.equal( 'foo bar' );
} );

it( 'should output three values if bottom is different then top', () => {
expect( getTopRightBottomLeftShorthandValue( { top: 'foo', right: 'foo', bottom: 'bar', left: 'foo' } ) )
expect( getBoxEdgesShorthandValue( { top: 'foo', right: 'foo', bottom: 'bar', left: 'foo' } ) )
.to.equal( 'foo foo bar' );
} );

it( 'should output four values if left is different then right', () => {
expect( getTopRightBottomLeftShorthandValue( { top: 'foo', right: 'foo', bottom: 'foo', left: 'bar' } ) )
expect( getBoxEdgesShorthandValue( { top: 'foo', right: 'foo', bottom: 'foo', left: 'bar' } ) )
.to.equal( 'foo foo foo bar' );
} );
} );

describe( 'getTopRightBottomLeftValues()', () => {
describe( 'getBoxEdgesValues()', () => {
it( 'should parse empty string', () => {
expect( getTopRightBottomLeftValues( '' ) ).to.deep.equal( {
expect( getBoxEdgesValues( '' ) ).to.deep.equal( {
top: undefined,
right: undefined,
bottom: undefined,
Expand All @@ -100,7 +100,7 @@ describe( 'Styles utils', () => {
} );

it( 'should parse one value', () => {
expect( getTopRightBottomLeftValues( 'foo' ) ).to.deep.equal( {
expect( getBoxEdgesValues( 'foo' ) ).to.deep.equal( {
top: 'foo',
right: 'foo',
bottom: 'foo',
Expand All @@ -109,7 +109,7 @@ describe( 'Styles utils', () => {
} );

it( 'should parse one value', () => {
expect( getTopRightBottomLeftValues( 'foo' ) ).to.deep.equal( {
expect( getBoxEdgesValues( 'foo' ) ).to.deep.equal( {
top: 'foo',
right: 'foo',
bottom: 'foo',
Expand All @@ -118,7 +118,7 @@ describe( 'Styles utils', () => {
} );

it( 'should parse two value', () => {
expect( getTopRightBottomLeftValues( 'foo bar' ) ).to.deep.equal( {
expect( getBoxEdgesValues( 'foo bar' ) ).to.deep.equal( {
top: 'foo',
right: 'bar',
bottom: 'foo',
Expand All @@ -127,7 +127,7 @@ describe( 'Styles utils', () => {
} );

it( 'should parse three values', () => {
expect( getTopRightBottomLeftValues( 'foo foo bar' ) ).to.deep.equal( {
expect( getBoxEdgesValues( 'foo foo bar' ) ).to.deep.equal( {
top: 'foo',
right: 'foo',
bottom: 'bar',
Expand All @@ -136,7 +136,7 @@ describe( 'Styles utils', () => {
} );

it( 'should output four values if left is different then right', () => {
expect( getTopRightBottomLeftValues( 'foo foo foo bar' ) ).to.deep.equal( {
expect( getBoxEdgesValues( 'foo foo foo bar' ) ).to.deep.equal( {
top: 'foo',
right: 'foo',
bottom: 'foo',
Expand Down
4 changes: 2 additions & 2 deletions tests/view/stylesmap.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import StylesMap, { StylesProcessor } from '../../src/view/stylesmap';
import encodedImage from './_utils/encodedimage.txt';
import { addPaddingRules } from '../../src/view/styles/padding';
import { getTopRightBottomLeftValueReducer } from '../../src/view/styles/utils';
import { getBoxEdgesValueReducer } from '../../src/view/styles/utils';

describe( 'StylesMap', () => {
let stylesMap, stylesProcessor;
Expand All @@ -23,7 +23,7 @@ describe( 'StylesMap', () => {
path: 'foo.top',
value
} ) );
stylesProcessor.setReducer( 'foo', getTopRightBottomLeftValueReducer( 'foo' ) );
stylesProcessor.setReducer( 'foo', getBoxEdgesValueReducer( 'foo' ) );

addPaddingRules( stylesProcessor );
StylesMap._setProcessor( stylesProcessor );
Expand Down