From 186e936efa3919245b73bd1cb7bbd5ff1c29ebe0 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Tue, 29 Jan 2019 15:32:41 +0100 Subject: [PATCH 1/7] remove global overrides --- docs/src/app.tsx | 11 +--- src/lib/fontSizeUtility.ts | 56 ++++--------------- src/lib/index.ts | 2 +- src/themes/teams/siteVariables.ts | 6 -- src/themes/teams/staticStyles/globalStyles.ts | 4 -- src/themes/types.ts | 1 - test/specs/lib/fontSizeUtility-test.ts | 26 +++------ 7 files changed, 21 insertions(+), 85 deletions(-) diff --git a/docs/src/app.tsx b/docs/src/app.tsx index 8eacc873d5..0cb2508165 100644 --- a/docs/src/app.tsx +++ b/docs/src/app.tsx @@ -1,7 +1,6 @@ import * as React from 'react' import { Provider, themes } from '@stardust-ui/react' -import { mergeThemes } from '../../src/lib' import { ThemeContext } from './context/ThemeContext' import Router from './routes' @@ -34,15 +33,7 @@ class App extends React.Component { const { themeName } = this.state return ( - + diff --git a/src/lib/fontSizeUtility.ts b/src/lib/fontSizeUtility.ts index 0950c2af28..ec6511d47a 100644 --- a/src/lib/fontSizeUtility.ts +++ b/src/lib/fontSizeUtility.ts @@ -1,15 +1,15 @@ import * as _ from 'lodash' import isBrowser from './isBrowser' -const DEFAULT_FONT_SIZE_IN_PX = 16 -const DEFAULT_REM_SIZE_IN_PX = 10 -let _htmlFontSizeInPx: number | null = null +const DEFAULT_REM_SIZE_IN_PX = 16 -const getComputedFontSize = (): number => { +let _documentRemSize: number | null = null + +const getDocumentRemSize = (): number => { return isBrowser() ? getFontSizeValue(getComputedStyle(document.documentElement).fontSize) || DEFAULT_REM_SIZE_IN_PX - : DEFAULT_FONT_SIZE_IN_PX + : DEFAULT_REM_SIZE_IN_PX } const getFontSizeValue = (size?: string | null): number | null => { @@ -19,55 +19,23 @@ const getFontSizeValue = (size?: string | null): number | null => { /** * Converts the provided px size to rem based on the default font size of 10px unless * the HTML font size has been previously defined with setHTMLFontSize(). - * @param {number} value The px value to convert to rem. + * @param {number} valueInPx The px value to convert to rem. * @example * // Returns '1rem' * pxToRem(10) * @returns {string} The value converted to the rem. */ -export const pxToRem = (value: number = 0): string => { - if (!_htmlFontSizeInPx) { - _htmlFontSizeInPx = getComputedFontSize() +export const pxToRem = (valueInPx: number = 0): string => { + if (!_documentRemSize) { + _documentRemSize = getDocumentRemSize() } if (process.env.NODE_ENV !== 'production') { - if (value < 0) { - throw new Error(`Invalid value of: '${value}'.`) + if (valueInPx < 0) { + throw new Error(`Invalid value of: '${valueInPx}'.`) } } - const convertedValueInRems = value / _htmlFontSizeInPx + const convertedValueInRems = valueInPx / _documentRemSize return `${_.round(convertedValueInRems, 4)}rem` } - -/** - * Sets the HTML font size for use for px to rem conversion. - * Providing null for fontSize will get the computed font size based on the document, or set it to DEFAULT_REM_SIZE_IN_PX. - * @param {string} [fontSize] The font size in px, to set as the HTML font size in the fontSizeUtility. - * @example - * // Sets the HTML font size to 10px. - * setHTMLFontSize('10px') - * @example - * // Sets the HTML font size based on document.fontSize. - * setHTMLFontSize() - */ -export const setHTMLFontSize = (fontSize?: string): void => { - if (!fontSize) { - throw new Error('fontSize is not defined') - } - - const htmlFontSizeValue = getFontSizeValue(fontSize) || 0 - const htmlFontSizeUnit = fontSize.replace(htmlFontSizeValue.toString(), '') - - if (process.env.NODE_ENV !== 'production') { - if (htmlFontSizeValue <= 0) { - throw new Error(`Invalid htmlFontSizeValue of: '${htmlFontSizeValue}'.`) - } - - if (htmlFontSizeUnit !== 'px') { - throw new Error(`Expected htmlFontSize to be in px, but got: '${htmlFontSizeUnit}'.`) - } - } - - _htmlFontSizeInPx = htmlFontSizeValue || getComputedFontSize() -} diff --git a/src/lib/index.ts b/src/lib/index.ts index f7067c9f0f..52a1f1fe2c 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -32,7 +32,7 @@ export { default as isBrowser } from './isBrowser' export { default as doesNodeContainClick } from './doesNodeContainClick' export { default as leven } from './leven' -export { pxToRem, setHTMLFontSize } from './fontSizeUtility' +export { pxToRem } from './fontSizeUtility' export { customPropTypes } export { default as createAnimationStyles } from './createAnimationStyles' export { default as createComponent } from './createStardustComponent' diff --git a/src/themes/teams/siteVariables.ts b/src/themes/teams/siteVariables.ts index f7cf0aac2f..52d311398c 100644 --- a/src/themes/teams/siteVariables.ts +++ b/src/themes/teams/siteVariables.ts @@ -1,11 +1,6 @@ import { pxToRem } from '../../lib' import { colors } from './colors' -// -// VARIABLES -// -export const htmlFontSize = '10px' // what 1rem represents - // // COLORS // @@ -96,7 +91,6 @@ export const bodyPadding = 0 export const bodyMargin = 0 export const bodyFontFamily = '"Segoe UI", "Helvetica Neue", "Apple Color Emoji", "Segoe UI Emoji", Helvetica, Arial, sans-serif' -export const bodyFontSize = '1.4rem' export const bodyBackground = white export const bodyColor = black export const bodyLineHeight = lineHeightMedium diff --git a/src/themes/teams/staticStyles/globalStyles.ts b/src/themes/teams/staticStyles/globalStyles.ts index fdec79ddbb..2081589575 100644 --- a/src/themes/teams/staticStyles/globalStyles.ts +++ b/src/themes/teams/staticStyles/globalStyles.ts @@ -1,14 +1,10 @@ import { StaticStyleFunction } from '../../types' const globalStyles: StaticStyleFunction = siteVars => ({ - html: { - fontSize: siteVars.htmlFontSize, - }, body: { padding: siteVars.bodyPadding, margin: siteVars.bodyMargin, fontFamily: siteVars.bodyFontFamily, - fontSize: siteVars.bodyFontSize, lineHeight: siteVars.bodyLineHeight, }, }) diff --git a/src/themes/types.ts b/src/themes/types.ts index 3882596ce5..0908dbb0f7 100644 --- a/src/themes/types.ts +++ b/src/themes/types.ts @@ -134,7 +134,6 @@ export interface SiteVariablesInput extends ObjectOf { emphasisColors?: EmphasisColors naturalColors?: NaturalColorsStrict brand?: string - htmlFontSize?: string } export interface SiteVariablesPrepared extends SiteVariablesInput { diff --git a/test/specs/lib/fontSizeUtility-test.ts b/test/specs/lib/fontSizeUtility-test.ts index 95310871ba..cc04a830b5 100644 --- a/test/specs/lib/fontSizeUtility-test.ts +++ b/test/specs/lib/fontSizeUtility-test.ts @@ -1,4 +1,4 @@ -import { pxToRem, setHTMLFontSize } from 'src/lib' +import { pxToRem } from 'src/lib' describe('fontSizeUtility', () => { describe('pxToRem', () => { @@ -10,19 +10,19 @@ describe('fontSizeUtility', () => { expect(() => pxToRem(-1)).toThrowError() }) - it('returns 1rem with base font size of 10px.', () => { - setHTMLFontSize('10px') + xit('returns 1rem with base font size of 10px.', () => { + // setHTMLFontSize('10px') expect(pxToRem(10)).toEqual('1rem') }) - it('returns 0.714rem with a base font size of 14px.', () => { - setHTMLFontSize('14px') + xit('returns 0.714rem with a base font size of 14px.', () => { + // setHTMLFontSize('14px') expect(pxToRem(10)).toEqual('0.7143rem') }) - it('returns 1.25rem with a base font size of 8px.', () => { - setHTMLFontSize('8px') + xit('returns 1.25rem with a base font size of 8px.', () => { + // setHTMLFontSize('8px') expect(pxToRem(10)).toEqual('1.25rem') }) @@ -35,16 +35,4 @@ describe('fontSizeUtility', () => { expect(pxToRem(0)).toEqual('0rem') }) }) - - describe('setHTMLFontSize', () => { - it('throws when htmlFontSize is in rems.', () => { - expect(() => setHTMLFontSize('8rem')).toThrowError() - }) - - it('throws when htmlFontSize is <= 0px.', () => { - expect(() => setHTMLFontSize('0px')).toThrowError() - - expect(() => setHTMLFontSize('-1px')).toThrowError() - }) - }) }) From 738280548ee01028d0ca376d2ea1fad7f4473a05 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Tue, 29 Jan 2019 16:28:44 +0100 Subject: [PATCH 2/7] fix pxToRem tests --- src/lib/fontSizeUtility.ts | 20 +++++++++--------- test/specs/lib/fontSizeUtility-test.ts | 29 +++++++++++--------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/lib/fontSizeUtility.ts b/src/lib/fontSizeUtility.ts index ec6511d47a..c8464f53f0 100644 --- a/src/lib/fontSizeUtility.ts +++ b/src/lib/fontSizeUtility.ts @@ -20,22 +20,22 @@ const getFontSizeValue = (size?: string | null): number | null => { * Converts the provided px size to rem based on the default font size of 10px unless * the HTML font size has been previously defined with setHTMLFontSize(). * @param {number} valueInPx The px value to convert to rem. + * @param {number} valueInPx Rem size to use for convertions. Optional - document's font size will be taken otherwise. * @example - * // Returns '1rem' - * pxToRem(10) + * // Returns '1rem' for default document font size (16px). + * pxToRem(16) + * + * // Returns '2rem'. + * pxToRem(32, 16) * @returns {string} The value converted to the rem. */ -export const pxToRem = (valueInPx: number = 0): string => { - if (!_documentRemSize) { +export const pxToRem = (valueInPx: number = 0, baseRemSize?: number): string => { + if (!baseRemSize && !_documentRemSize) { _documentRemSize = getDocumentRemSize() } - if (process.env.NODE_ENV !== 'production') { - if (valueInPx < 0) { - throw new Error(`Invalid value of: '${valueInPx}'.`) - } - } - const convertedValueInRems = valueInPx / _documentRemSize + const remSize = baseRemSize || _documentRemSize || DEFAULT_REM_SIZE_IN_PX + const convertedValueInRems = valueInPx / remSize return `${_.round(convertedValueInRems, 4)}rem` } diff --git a/test/specs/lib/fontSizeUtility-test.ts b/test/specs/lib/fontSizeUtility-test.ts index cc04a830b5..69a103afa6 100644 --- a/test/specs/lib/fontSizeUtility-test.ts +++ b/test/specs/lib/fontSizeUtility-test.ts @@ -2,29 +2,20 @@ import { pxToRem } from 'src/lib' describe('fontSizeUtility', () => { describe('pxToRem', () => { - it('returns 1rem for 10px with a default HTML font size of 10px.', () => { - expect(pxToRem(10)).toEqual('1rem') + it('returns 1rem for 16px with a default HTML font size of 16px.', () => { + expect(pxToRem(16)).toEqual('1rem') }) - it('should throw error when called with a negative number.', () => { - expect(() => pxToRem(-1)).toThrowError() + it('returns 1rem with base font size of 10px.', () => { + expect(pxToRem(10, 10)).toEqual('1rem') }) - xit('returns 1rem with base font size of 10px.', () => { - // setHTMLFontSize('10px') - expect(pxToRem(10)).toEqual('1rem') + it('returns 0.714rem with a base font size of 14px.', () => { + expect(pxToRem(10, 14)).toEqual('0.7143rem') }) - xit('returns 0.714rem with a base font size of 14px.', () => { - // setHTMLFontSize('14px') - - expect(pxToRem(10)).toEqual('0.7143rem') - }) - - xit('returns 1.25rem with a base font size of 8px.', () => { - // setHTMLFontSize('8px') - - expect(pxToRem(10)).toEqual('1.25rem') + it('returns 1.25rem with a base font size of 8px.', () => { + expect(pxToRem(10, 8)).toEqual('1.25rem') }) it('returns 0rem when pxToRem is called without a value.', () => { @@ -34,5 +25,9 @@ describe('fontSizeUtility', () => { it('returns 0rem when pxToRem is called with 0.', () => { expect(pxToRem(0)).toEqual('0rem') }) + + it('should handle negative input values and return negative conversion result.', () => { + expect(pxToRem(-16, 16)).toEqual('-1rem') + }) }) }) From 024978e13d41918dd9aa9c2f1690dd3eb7f66f4b Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Wed, 30 Jan 2019 11:47:43 +0100 Subject: [PATCH 3/7] fix js docs --- src/lib/fontSizeUtility.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/fontSizeUtility.ts b/src/lib/fontSizeUtility.ts index c8464f53f0..afb9cba773 100644 --- a/src/lib/fontSizeUtility.ts +++ b/src/lib/fontSizeUtility.ts @@ -19,8 +19,8 @@ const getFontSizeValue = (size?: string | null): number | null => { /** * Converts the provided px size to rem based on the default font size of 10px unless * the HTML font size has been previously defined with setHTMLFontSize(). - * @param {number} valueInPx The px value to convert to rem. - * @param {number} valueInPx Rem size to use for convertions. Optional - document's font size will be taken otherwise. + * @param {number} valueInPx - The px value to convert to rem. + * @param {number} baseRemSize - Rem size to use for convertions. Optional - document's font size will be taken otherwise. * @example * // Returns '1rem' for default document font size (16px). * pxToRem(16) @@ -29,7 +29,7 @@ const getFontSizeValue = (size?: string | null): number | null => { * pxToRem(32, 16) * @returns {string} The value converted to the rem. */ -export const pxToRem = (valueInPx: number = 0, baseRemSize?: number): string => { +export const pxToRem = (valueInPx: number, baseRemSize?: number): string => { if (!baseRemSize && !_documentRemSize) { _documentRemSize = getDocumentRemSize() } From 412a9eefdf3b4934454d6cfdde46194bfe1c8939 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Wed, 30 Jan 2019 13:47:43 +0100 Subject: [PATCH 4/7] revert back font size setting for Teams theme --- docs/src/app.tsx | 11 ++++++++++- src/themes/teams/siteVariables.ts | 5 +++++ src/themes/teams/staticStyles/globalStyles.ts | 4 ++++ src/themes/types.ts | 1 + test/specs/lib/fontSizeUtility-test.ts | 4 ---- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/src/app.tsx b/docs/src/app.tsx index 0cb2508165..8eacc873d5 100644 --- a/docs/src/app.tsx +++ b/docs/src/app.tsx @@ -1,6 +1,7 @@ import * as React from 'react' import { Provider, themes } from '@stardust-ui/react' +import { mergeThemes } from '../../src/lib' import { ThemeContext } from './context/ThemeContext' import Router from './routes' @@ -33,7 +34,15 @@ class App extends React.Component { const { themeName } = this.state return ( - + diff --git a/src/themes/teams/siteVariables.ts b/src/themes/teams/siteVariables.ts index 52d311398c..4d786d5c5c 100644 --- a/src/themes/teams/siteVariables.ts +++ b/src/themes/teams/siteVariables.ts @@ -1,6 +1,11 @@ import { pxToRem } from '../../lib' import { colors } from './colors' +// +// VARIABLES +// +export const htmlFontSize = '10px' // what 1rem represents + // // COLORS // diff --git a/src/themes/teams/staticStyles/globalStyles.ts b/src/themes/teams/staticStyles/globalStyles.ts index 2081589575..fdec79ddbb 100644 --- a/src/themes/teams/staticStyles/globalStyles.ts +++ b/src/themes/teams/staticStyles/globalStyles.ts @@ -1,10 +1,14 @@ import { StaticStyleFunction } from '../../types' const globalStyles: StaticStyleFunction = siteVars => ({ + html: { + fontSize: siteVars.htmlFontSize, + }, body: { padding: siteVars.bodyPadding, margin: siteVars.bodyMargin, fontFamily: siteVars.bodyFontFamily, + fontSize: siteVars.bodyFontSize, lineHeight: siteVars.bodyLineHeight, }, }) diff --git a/src/themes/types.ts b/src/themes/types.ts index 0908dbb0f7..3882596ce5 100644 --- a/src/themes/types.ts +++ b/src/themes/types.ts @@ -134,6 +134,7 @@ export interface SiteVariablesInput extends ObjectOf { emphasisColors?: EmphasisColors naturalColors?: NaturalColorsStrict brand?: string + htmlFontSize?: string } export interface SiteVariablesPrepared extends SiteVariablesInput { diff --git a/test/specs/lib/fontSizeUtility-test.ts b/test/specs/lib/fontSizeUtility-test.ts index 69a103afa6..d111610c70 100644 --- a/test/specs/lib/fontSizeUtility-test.ts +++ b/test/specs/lib/fontSizeUtility-test.ts @@ -18,10 +18,6 @@ describe('fontSizeUtility', () => { expect(pxToRem(10, 8)).toEqual('1.25rem') }) - it('returns 0rem when pxToRem is called without a value.', () => { - expect(pxToRem()).toEqual('0rem') - }) - it('returns 0rem when pxToRem is called with 0.', () => { expect(pxToRem(0)).toEqual('0rem') }) From c3f5eaef260de039360afdb9f3cae5064d82f0a6 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Wed, 30 Jan 2019 15:36:37 +0100 Subject: [PATCH 5/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2618309eed..cfcbea128a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Features - Export `triangle-left` and `triangle-right` icons in Teams theme @codepretty ([#785](https://github.com/stardust-ui/react/pull/785)) +- Remove ability to introduce global style changes for HTML document from `pxToRem` @kuzhelov ([#789](https://github.com/stardust-ui/react/pull/789)) ### Fixes - Handle `onClick` and `onFocus` on ListItems correctly @layershifter ([#779](https://github.com/stardust-ui/react/pull/779)) From 0b9ec7de15c5d7c8d1c26cd52e4a72d7f714f7e8 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Wed, 30 Jan 2019 15:41:13 +0100 Subject: [PATCH 6/7] revert site variables changes --- src/themes/teams/siteVariables.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/themes/teams/siteVariables.ts b/src/themes/teams/siteVariables.ts index 4d786d5c5c..f7cf0aac2f 100644 --- a/src/themes/teams/siteVariables.ts +++ b/src/themes/teams/siteVariables.ts @@ -96,6 +96,7 @@ export const bodyPadding = 0 export const bodyMargin = 0 export const bodyFontFamily = '"Segoe UI", "Helvetica Neue", "Apple Color Emoji", "Segoe UI Emoji", Helvetica, Arial, sans-serif' +export const bodyFontSize = '1.4rem' export const bodyBackground = white export const bodyColor = black export const bodyLineHeight = lineHeightMedium From 8221e90771dfaa57c9bab77fda2a79e4e5c64830 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Wed, 30 Jan 2019 16:31:45 +0100 Subject: [PATCH 7/7] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d039b2bf1..993ff245e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Export `triangle-down` and `triangle-right` icons in Teams theme @codepretty ([#785](https://github.com/stardust-ui/react/pull/785)) - Add RTL examples for `Button` and `Divider` components @mnajdova ([#792](https://github.com/stardust-ui/react/pull/792)) - Add mechanism for marking icons that should rotate in RTL in Teams theme; marked icons: `send`, `bullets`, `leave`, `outdent`, `redo`, `undo`, `send` @mnajdova ([#788](https://github.com/stardust-ui/react/pull/788)) -- Remove ability to introduce global style changes for HTML document from `pxToRem` @kuzhelov ([#789](https://github.com/stardust-ui/react/pull/789)) +- Remove ability to introduce global style overrides for HTML document from `pxToRem` @kuzhelov ([#789](https://github.com/stardust-ui/react/pull/789)) ### Fixes - Handle `onClick` and `onFocus` on ListItems correctly @layershifter ([#779](https://github.com/stardust-ui/react/pull/779))