Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WRQ-11247: Added fontScale prop for large text mode scale values #3217

Draft
wants to merge 12 commits into
base: develop
Choose a base branch
from
34 changes: 31 additions & 3 deletions packages/ui/resolution/ResolutionDecorator.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import PropTypes from 'prop-types';
import hoc from '@enact/core/hoc';

import {init, config as riConfig, defineScreenTypes, getResolutionClasses} from './resolution';
import {init, calculateFontSize, config as riConfig, defineScreenTypes, getResolutionClasses, updateBaseFontSize, updateFontScale} from './resolution';

/**
* Default config for `ResolutionDecorator`.
Expand Down Expand Up @@ -97,7 +97,22 @@
static displayName = 'ResolutionDecorator';

static propTypes = /** @lends ui/resolution.ResolutionDecorator.prototype */ {
className: PropTypes.string
className: PropTypes.string,

/**
* Font Scale for large text mode.
* Use this value to set the scale of the font.
* This is the value that will be multiplied by pxPerRem, which is determined by resolution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* Font Scale for large text mode.
* Use this value to set the scale of the font.
* This is the value that will be multiplied by pxPerRem, which is determined by resolution.
/**
* Font scale value for the large text mode.
* Use this value to set the scale of the font.
* This is the value that will be multiplied by pxPerRem, which is determined by the resolution.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think large text mode word may be confused with the original large text mode that is set in ThemeDecorator.

Copy link
Author

Choose a reason for hiding this comment

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

I used it because the currently used term is large text mode. It may be confused with large text in ThemeDecorator, but since there is currently no fixed name, it would be a good idea to change it after the name is fixed. This is a draft and will not be merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. I think we need to set a proper name for this mode when merging it. I will approve it since this is a draft PR.

*
* @type {Number}
* @default 1
* @public
*/
fontScale: PropTypes.number
juwonjeong marked this conversation as resolved.
Show resolved Hide resolved
};

static defaultProps = {
fontScale: 1
};

constructor (props) {
Expand All @@ -114,6 +129,15 @@
if (config.dynamic) window.addEventListener('resize', this.handleResize);
// eslint-disable-next-line react/no-find-dom-node
this.rootNode = ReactDOM.findDOMNode(this);

updateFontScale(this.props.fontScale);
juwonjeong marked this conversation as resolved.
Show resolved Hide resolved
}

componentDidUpdate (prevProps) {

Check warning on line 136 in packages/ui/resolution/ResolutionDecorator.js

View check run for this annotation

Codecov / codecov/patch

packages/ui/resolution/ResolutionDecorator.js#L136

Added line #L136 was not covered by tests
if (prevProps.fontScale !== this.props.fontScale) {
updateFontScale(this.props.fontScale);
updateBaseFontSize(calculateFontSize());

Check warning on line 139 in packages/ui/resolution/ResolutionDecorator.js

View check run for this annotation

Codecov / codecov/patch

packages/ui/resolution/ResolutionDecorator.js#L138-L139

Added lines #L138 - L139 were not covered by tests
}
}

componentWillUnmount () {
Expand Down Expand Up @@ -145,11 +169,15 @@
}

render () {
const {...rest} = this.props;

delete rest.fontScale;

// Check if the classes are different from our previous classes
let classes = getResolutionClasses();

if (this.props.className) classes += (classes ? ' ' : '') + this.props.className;
return <Wrapped {...this.props} className={classes} />;
return <Wrapped {...rest} className={classes} />;
}
};
});
Expand Down
22 changes: 18 additions & 4 deletions packages/ui/resolution/resolution.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
let baseScreen,
fontScale = 1,
orientation,
riRatio,
screenType,
Expand Down Expand Up @@ -201,11 +202,11 @@
let size;

if (orientation === 'portrait' && config.orientationHandling === 'scale') {
size = scrObj.height / scrObj.width * scrObj.pxPerRem;
size = scrObj.height / scrObj.width * (scrObj.pxPerRem * fontScale);

Check warning on line 205 in packages/ui/resolution/resolution.js

View check run for this annotation

Codecov / codecov/patch

packages/ui/resolution/resolution.js#L205

Added line #L205 was not covered by tests
} else {
size = scrObj.pxPerRem;
size = (scrObj.pxPerRem * fontScale);
if (orientation === 'landscape' && shouldScaleFontSize) {
size = parseInt(workspaceBounds.height * scrObj.pxPerRem / scrObj.height);
size = parseInt(workspaceBounds.height * (scrObj.pxPerRem * fontScale) / scrObj.height);

Check warning on line 209 in packages/ui/resolution/resolution.js

View check run for this annotation

Codecov / codecov/patch

packages/ui/resolution/resolution.js#L209

Added line #L209 was not covered by tests
}
}
return size + 'px';
Expand All @@ -224,6 +225,17 @@
}
}

/**
* @function
* @memberof ui/resolution
* @param {Number} scale A value is used multiflied scale to base font size.
juwonjeong marked this conversation as resolved.
Show resolved Hide resolved
* @returns {undefined}
* @private
*/
function updateFontScale (fontScaleValue) {
fontScale = fontScaleValue;
}

/**
* Returns the CSS classes for the given `type`.
*
Expand Down Expand Up @@ -494,5 +506,7 @@
scaleToRem,
selectSrc,
unit,
unitToPixelFactors
unitToPixelFactors,
updateBaseFontSize,
updateFontScale
};