From 037bd41d2f03cf4e2608adecd5de4206a215ee9d Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:46:51 +0300 Subject: [PATCH 01/17] Added Star/StarRatings Component. --- StarRatings/Star.js | 116 +++++++++++ StarRatings/Star.module.less | 31 +++ StarRatings/StarRatings.js | 194 ++++++++++++++++++ StarRatings/StarRatings.module.less | 16 ++ StarRatings/package.json | 3 + samples/sampler/stories/default/Star.js | 35 ++++ .../sampler/stories/default/StarRatings.js | 35 ++++ 7 files changed, 430 insertions(+) create mode 100644 StarRatings/Star.js create mode 100644 StarRatings/Star.module.less create mode 100644 StarRatings/StarRatings.js create mode 100644 StarRatings/StarRatings.module.less create mode 100644 StarRatings/package.json create mode 100644 samples/sampler/stories/default/Star.js create mode 100644 samples/sampler/stories/default/StarRatings.js diff --git a/StarRatings/Star.js b/StarRatings/Star.js new file mode 100644 index 000000000..d51566eaa --- /dev/null +++ b/StarRatings/Star.js @@ -0,0 +1,116 @@ +import {adaptEvent, forProp, forward, handle} from '@enact/core/handle'; +import kind from '@enact/core/kind'; +import Spottable from '@enact/spotlight/Spottable'; +import PropTypes from 'prop-types'; +import compose from 'ramda/src/compose'; + +import Icon from '../Icon'; +import Skinnable from '../Skinnable'; + +import css from './Star.module.less'; + +const SpottableIcon = Spottable(Icon); + +const StarBase = kind({ + name: 'Star', + + propTypes: { + /** + * Unique identifier for the star. + * + * @type {String} + * @required + * @public + */ + id: PropTypes.string.isRequired, + + /** + * When `true`, the star is focusable and clickable. + * + * @type {Boolean} + * @default false + * @public + */ + active: PropTypes.bool, + + /** + * When `true`, the star glows while its row has focus. + * + * @type {Boolean} + * @default false + * @public + */ + highlighted: PropTypes.bool, + + /** + * Fill amount from 0 to 1, rounded to 10% steps. + * + * @type {Number} + * @default 0 + * @public + */ + value: PropTypes.number + }, + + defaultProps: { + active: false, + highlighted: false, + value: 0 + }, + + styles: { + css, + className: 'star' + }, + + handlers: { + onClick: handle( + forProp('active', true), + adaptEvent( + (ev, {id}) => ({id, ev}), + forward('onClick') + ) + ) + }, + + computed: { + className: ({highlighted, styler}) => styler.append({highlighted}), + style: ({style, value}) => { + const percent = Math.round(value * 10) * 10; + return {...style, '--star-fill': percent + '%'}; + } + }, + + render: ({active, className, id, onClick, style, ...rest}) => { + delete rest.highlighted; + delete rest.value; + + const IconComponent = active ? SpottableIcon : Icon; + + return ( + + star + + ); + } +}); + +const StarDecorator = compose( + Skinnable +); + +const Star = StarDecorator(StarBase); + +export default Star; +export { + Star, + StarBase, + StarDecorator +}; \ No newline at end of file diff --git a/StarRatings/Star.module.less b/StarRatings/Star.module.less new file mode 100644 index 000000000..bab744cad --- /dev/null +++ b/StarRatings/Star.module.less @@ -0,0 +1,31 @@ +@import "../styles/mixins.less"; +@import "../styles/variables.less"; +@import "../styles/skin.less"; + +.star { + transition: filter 200ms ease-out; + + // Touch handling + touch-action: manipulation; // no double-tap-zoom delay + -webkit-tap-highlight-color: transparent; // no grey flash on tap + -webkit-user-select: none; // a long press won't select the glyph + user-select: none; + -webkit-touch-callout: none; // no long-press context menu + + .applySkins({ + background: linear-gradient( + to right, + var(--semantic-color-on-surface-main) var(--star-fill, 0%), + var(--semantic-color-surface-toggle) var(--star-fill, 0%) + ); + -webkit-background-clip: text; + background-clip: text; + color: transparent; + }); +} + +// When any star in the row has focus (remote, tap or drag), +// every highlighted star glows +:focus-within > .star.highlighted { + filter: drop-shadow(0 0 18px rgba(255, 255, 255, 0.8)); +} \ No newline at end of file diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js new file mode 100644 index 000000000..c821ba76a --- /dev/null +++ b/StarRatings/StarRatings.js @@ -0,0 +1,194 @@ +import {adaptEvent, forProp, forward, handle} from '@enact/core/handle'; +import kind from '@enact/core/kind'; +import Spotlight from '@enact/spotlight'; +import SpotlightContainerDecorator from '@enact/spotlight/SpotlightContainerDecorator'; +import Changeable from '@enact/ui/Changeable'; +import PropTypes from 'prop-types'; +import compose from 'ramda/src/compose'; + +import Skinnable from '../Skinnable'; + +import Star from './Star'; + +import css from './StarRatings.module.less'; + +// Returns the star element under the current touch point, or null +const getStarFromTouch = (ev, id) => { + const touch = ev.touches && ev.touches[0]; + if (!touch) return null; + + const el = document.elementFromPoint(touch.clientX, touch.clientY); + return el && el.closest(`[id^="${id}-star-"]`); +}; + +// Reads the star number (1-based) from a star id like `${id}-star-3` +const getStarNumber = (starId, id) => Number(starId.slice(`${id}-star-`.length)); + +// Moves focus to the star, which triggers the :focus-within glow +const focusStar = (star) => { + if (star && document.activeElement !== star) { + if (!Spotlight.focus(star)) { + star.focus({preventScroll: true}); // fallback if Spotlight declines + } + } +}; + +const StarRatingsBase = kind({ + name: 'StarRatings', + + propTypes: { + /** + * Unique identifier, used to build each star's id. + * + * @type {String} + * @required + * @public + */ + id: PropTypes.string.isRequired, + + /** + * `false` = display mode (shows `displayValue`, not interactive) + * `true` = edit mode (shows `value`, interactive) + * + * @type {Boolean} + * @default false + * @public + */ + active: PropTypes.bool, + + /** + * Value shown in display mode. Supports fractions, e.g. 3.7. + * + * @type {Number} + * @default 0 + * @public + */ + displayValue: PropTypes.number, + + /** + * Number of stars. + * + * @type {Number} + * @default 5 + * @public + */ + max: PropTypes.number, + + /** + * Called in edit mode with `{value}`: the number of the selected star. + * + * @type {Function} + * @public + */ + onChange: PropTypes.func, + + /** + * Value shown in edit mode. If omitted, the component keeps its own + * internal value (via Changeable). Use `defaultValue` to set a starting value. + * + * @type {Number} + * @public + */ + value: PropTypes.number + }, + + defaultProps: { + active: false, + displayValue: 0, + max: 5 + }, + + styles: { + css, + className: 'starRatings' + }, + + handlers: { + // Star forwards {id, ev}; turn it into {value} for onChange + onStarClick: handle( + forProp('active', true), + adaptEvent( + ({id: starId}, {id}) => ({value: getStarNumber(starId, id)}), + forward('onChange') + ) + ), + + // Glow starts as soon as the finger touches a star + onTouchStart: handle( + forward('onTouchStart'), + forProp('active', true), + (ev, {id}) => { + focusStar(getStarFromTouch(ev, id)); + return true; + } + ), + + // Drag across the stars to change the rating + onTouchMove: handle( + forward('onTouchMove'), + forProp('active', true), + (ev, {id, onChange, value}) => { + const star = getStarFromTouch(ev, id); + if (!star) return false; // outside the row: keep the last value and focus + + focusStar(star); + + const starNumber = getStarNumber(star.id, id); + if (onChange && starNumber && starNumber !== value) { + onChange({value: starNumber}); + } + return true; + } + ) + }, + + computed: { + className: ({active, styler}) => styler.append({active}), + shownValue: ({active, displayValue, value}) => (active ? value : displayValue) || 0 + }, + + render: ({active, id, max, onStarClick, onTouchMove, onTouchStart, shownValue, ...rest}) => { + delete rest.displayValue; + delete rest.onChange; + delete rest.value; + + return ( +
+ {Array.from({length: max}, (_, i) => { + const starValue = Math.min(Math.max(shownValue - i, 0), 1); + + return ( + 0} + value={starValue} + onClick={onStarClick} + /> + ); + })} +
+ ); + } +}); + +const StarRatingsDecorator = compose( + Changeable, + SpotlightContainerDecorator({enterTo: 'last-focused'}), + Skinnable +); + +const StarRatings = StarRatingsDecorator(StarRatingsBase); + +export default StarRatings; +export { + StarRatings, + StarRatingsBase, + StarRatingsDecorator +}; \ No newline at end of file diff --git a/StarRatings/StarRatings.module.less b/StarRatings/StarRatings.module.less new file mode 100644 index 000000000..f785f2e1e --- /dev/null +++ b/StarRatings/StarRatings.module.less @@ -0,0 +1,16 @@ +.starRatings { + display: inline-flex; + align-items: center; + -webkit-user-select: none; + user-select: none; + + // In edit mode, horizontal finger movement drives the rating + // instead of scrolling; vertical page scrolling still works + &.active { + touch-action: pan-y; + } + + > * { + margin: 0 12px; + } +} \ No newline at end of file diff --git a/StarRatings/package.json b/StarRatings/package.json new file mode 100644 index 000000000..9705af3f6 --- /dev/null +++ b/StarRatings/package.json @@ -0,0 +1,3 @@ +{ + "main": "StarRatings.js" +} \ No newline at end of file diff --git a/samples/sampler/stories/default/Star.js b/samples/sampler/stories/default/Star.js new file mode 100644 index 000000000..58c198016 --- /dev/null +++ b/samples/sampler/stories/default/Star.js @@ -0,0 +1,35 @@ +import Star, {StarBase} from '../../../../StarRatings/Star'; +import {mergeComponentMetadata} from '@enact/storybook-utils'; +import {action} from '@enact/storybook-utils/addons/actions'; +import {boolean, range} from '@enact/storybook-utils/addons/controls'; + +const StarConfig = mergeComponentMetadata('Star', Star, StarBase); + +export default { + title: 'Limestone/StarRatings', + component: 'Star' +}; + +export const _Star = (args) => { + const actions = { + onClick: action('onClick'), + }; + + const controls = { + active: args['active'], + value: args['value'] + }; + + return (<> + + ); +} + +boolean('active', _Star, StarConfig); +range('value', _Star, StarConfig, {min: 0, max: 1, step: 0.01}); + +_Star.storyName = 'Star'; \ No newline at end of file diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js new file mode 100644 index 000000000..b2e21def5 --- /dev/null +++ b/samples/sampler/stories/default/StarRatings.js @@ -0,0 +1,35 @@ +import {mergeComponentMetadata} from '@enact/storybook-utils'; +import {action} from '@enact/storybook-utils/addons/actions'; +import {boolean, range} from '@enact/storybook-utils/addons/controls'; +import StarRatings, {StarRatingsBase} from '../../../../StarRatings/StarRatings'; + +const StarRatingsConfig = mergeComponentMetadata('Star', StarRatings, StarRatingsBase); + +export default { + title: 'Limestone/StarRatings', + component: 'StarRatings' +}; + +export const _StarRatings = (args) => { + const actions = { + onChange: action('onChange') + }; + + const controls = { + active: args['active'], + displayValue: args['displayValue'] + }; + + return (<> + + ); +} + +boolean('active', _StarRatings, StarRatingsConfig); +range('displayValue', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}); + +_StarRatings.storyName = 'StarRatings'; \ No newline at end of file From f547d0a675457c3c66ada9b93cd32159d2974ca6 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Mon, 12 Oct 2026 10:18:37 +0300 Subject: [PATCH 02/17] Added sampler values. --- samples/sampler/stories/default/Star.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samples/sampler/stories/default/Star.js b/samples/sampler/stories/default/Star.js index 58c198016..0c4812a37 100644 --- a/samples/sampler/stories/default/Star.js +++ b/samples/sampler/stories/default/Star.js @@ -17,6 +17,7 @@ export const _Star = (args) => { const controls = { active: args['active'], + highlighted: args['highlighted'], value: args['value'] }; @@ -30,6 +31,7 @@ export const _Star = (args) => { } boolean('active', _Star, StarConfig); +boolean('highlighted', _Star, StarConfig); range('value', _Star, StarConfig, {min: 0, max: 1, step: 0.01}); _Star.storyName = 'Star'; \ No newline at end of file From 5bc89b696060b306243d62c0e6631b13671398c0 Mon Sep 17 00:00:00 2001 From: daniel-stoian-lgp Date: Thu, 15 Oct 2026 13:56:12 +0300 Subject: [PATCH 03/17] fixed lint --- StarRatings/Star.js | 130 +++++----- StarRatings/StarRatings.js | 232 +++++++++--------- samples/sampler/stories/default/Star.js | 38 +-- .../sampler/stories/default/StarRatings.js | 36 +-- 4 files changed, 218 insertions(+), 218 deletions(-) diff --git a/StarRatings/Star.js b/StarRatings/Star.js index d51566eaa..6e31ae896 100644 --- a/StarRatings/Star.js +++ b/StarRatings/Star.js @@ -12,105 +12,105 @@ import css from './Star.module.less'; const SpottableIcon = Spottable(Icon); const StarBase = kind({ - name: 'Star', + name: 'Star', - propTypes: { - /** + propTypes: { + /** * Unique identifier for the star. * * @type {String} * @required * @public */ - id: PropTypes.string.isRequired, + id: PropTypes.string.isRequired, - /** + /** * When `true`, the star is focusable and clickable. * * @type {Boolean} * @default false * @public */ - active: PropTypes.bool, + active: PropTypes.bool, - /** + /** * When `true`, the star glows while its row has focus. * * @type {Boolean} * @default false * @public */ - highlighted: PropTypes.bool, + highlighted: PropTypes.bool, - /** + /** * Fill amount from 0 to 1, rounded to 10% steps. * * @type {Number} * @default 0 * @public */ - value: PropTypes.number - }, - - defaultProps: { - active: false, - highlighted: false, - value: 0 - }, - - styles: { - css, - className: 'star' - }, - - handlers: { - onClick: handle( - forProp('active', true), - adaptEvent( - (ev, {id}) => ({id, ev}), - forward('onClick') - ) - ) - }, - - computed: { - className: ({highlighted, styler}) => styler.append({highlighted}), - style: ({style, value}) => { - const percent = Math.round(value * 10) * 10; - return {...style, '--star-fill': percent + '%'}; - } - }, - - render: ({active, className, id, onClick, style, ...rest}) => { - delete rest.highlighted; - delete rest.value; - - const IconComponent = active ? SpottableIcon : Icon; - - return ( - - star - - ); - } + value: PropTypes.number + }, + + defaultProps: { + active: false, + highlighted: false, + value: 0 + }, + + styles: { + css, + className: 'star' + }, + + handlers: { + onClick: handle( + forProp('active', true), + adaptEvent( + (ev, {id}) => ({id, ev}), + forward('onClick') + ) + ) + }, + + computed: { + className: ({highlighted, styler}) => styler.append({highlighted}), + style: ({style, value}) => { + const percent = Math.round(value * 10) * 10; + return {...style, '--star-fill': percent + '%'}; + } + }, + + render: ({active, className, id, onClick, style, ...rest}) => { + delete rest.highlighted; + delete rest.value; + + const IconComponent = active ? SpottableIcon : Icon; + + return ( + + star + + ); + } }); const StarDecorator = compose( - Skinnable + Skinnable ); const Star = StarDecorator(StarBase); export default Star; export { - Star, - StarBase, - StarDecorator -}; \ No newline at end of file + Star, + StarBase, + StarDecorator +}; diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index c821ba76a..901d764b6 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -14,11 +14,11 @@ import css from './StarRatings.module.less'; // Returns the star element under the current touch point, or null const getStarFromTouch = (ev, id) => { - const touch = ev.touches && ev.touches[0]; - if (!touch) return null; + const touch = ev.touches && ev.touches[0]; + if (!touch) return null; - const el = document.elementFromPoint(touch.clientX, touch.clientY); - return el && el.closest(`[id^="${id}-star-"]`); + const el = document.elementFromPoint(touch.clientX, touch.clientY); + return el && el.closest(`[id^="${id}-star-"]`); }; // Reads the star number (1-based) from a star id like `${id}-star-3` @@ -26,27 +26,27 @@ const getStarNumber = (starId, id) => Number(starId.slice(`${id}-star-`.length)) // Moves focus to the star, which triggers the :focus-within glow const focusStar = (star) => { - if (star && document.activeElement !== star) { - if (!Spotlight.focus(star)) { - star.focus({preventScroll: true}); // fallback if Spotlight declines - } - } + if (star && document.activeElement !== star) { + if (!Spotlight.focus(star)) { + star.focus({preventScroll: true}); // fallback if Spotlight declines + } + } }; const StarRatingsBase = kind({ - name: 'StarRatings', + name: 'StarRatings', - propTypes: { - /** + propTypes: { + /** * Unique identifier, used to build each star's id. * * @type {String} * @required * @public */ - id: PropTypes.string.isRequired, + id: PropTypes.string.isRequired, - /** + /** * `false` = display mode (shows `displayValue`, not interactive) * `true` = edit mode (shows `value`, interactive) * @@ -54,141 +54,141 @@ const StarRatingsBase = kind({ * @default false * @public */ - active: PropTypes.bool, + active: PropTypes.bool, - /** + /** * Value shown in display mode. Supports fractions, e.g. 3.7. * * @type {Number} * @default 0 * @public */ - displayValue: PropTypes.number, + displayValue: PropTypes.number, - /** + /** * Number of stars. * * @type {Number} * @default 5 * @public */ - max: PropTypes.number, + max: PropTypes.number, - /** + /** * Called in edit mode with `{value}`: the number of the selected star. * * @type {Function} * @public */ - onChange: PropTypes.func, + onChange: PropTypes.func, - /** + /** * Value shown in edit mode. If omitted, the component keeps its own * internal value (via Changeable). Use `defaultValue` to set a starting value. * * @type {Number} * @public */ - value: PropTypes.number - }, - - defaultProps: { - active: false, - displayValue: 0, - max: 5 - }, - - styles: { - css, - className: 'starRatings' - }, - - handlers: { - // Star forwards {id, ev}; turn it into {value} for onChange - onStarClick: handle( - forProp('active', true), - adaptEvent( - ({id: starId}, {id}) => ({value: getStarNumber(starId, id)}), - forward('onChange') - ) - ), - - // Glow starts as soon as the finger touches a star - onTouchStart: handle( - forward('onTouchStart'), - forProp('active', true), - (ev, {id}) => { - focusStar(getStarFromTouch(ev, id)); - return true; - } - ), - - // Drag across the stars to change the rating - onTouchMove: handle( - forward('onTouchMove'), - forProp('active', true), - (ev, {id, onChange, value}) => { - const star = getStarFromTouch(ev, id); - if (!star) return false; // outside the row: keep the last value and focus - - focusStar(star); - - const starNumber = getStarNumber(star.id, id); - if (onChange && starNumber && starNumber !== value) { - onChange({value: starNumber}); - } - return true; - } - ) - }, - - computed: { - className: ({active, styler}) => styler.append({active}), - shownValue: ({active, displayValue, value}) => (active ? value : displayValue) || 0 - }, - - render: ({active, id, max, onStarClick, onTouchMove, onTouchStart, shownValue, ...rest}) => { - delete rest.displayValue; - delete rest.onChange; - delete rest.value; - - return ( -
- {Array.from({length: max}, (_, i) => { - const starValue = Math.min(Math.max(shownValue - i, 0), 1); - - return ( - 0} - value={starValue} - onClick={onStarClick} - /> - ); - })} -
- ); - } + value: PropTypes.number + }, + + defaultProps: { + active: false, + displayValue: 0, + max: 5 + }, + + styles: { + css, + className: 'starRatings' + }, + + handlers: { + // Star forwards {id, ev}; turn it into {value} for onChange + onStarClick: handle( + forProp('active', true), + adaptEvent( + ({id: starId}, {id}) => ({value: getStarNumber(starId, id)}), + forward('onChange') + ) + ), + + // Glow starts as soon as the finger touches a star + onTouchStart: handle( + forward('onTouchStart'), + forProp('active', true), + (ev, {id}) => { + focusStar(getStarFromTouch(ev, id)); + return true; + } + ), + + // Drag across the stars to change the rating + onTouchMove: handle( + forward('onTouchMove'), + forProp('active', true), + (ev, {id, onChange, value}) => { + const star = getStarFromTouch(ev, id); + if (!star) return false; // outside the row: keep the last value and focus + + focusStar(star); + + const starNumber = getStarNumber(star.id, id); + if (onChange && starNumber && starNumber !== value) { + onChange({value: starNumber}); + } + return true; + } + ) + }, + + computed: { + className: ({active, styler}) => styler.append({active}), + shownValue: ({active, displayValue, value}) => (active ? value : displayValue) || 0 + }, + + render: ({active, id, max, onStarClick, onTouchMove, onTouchStart, shownValue, ...rest}) => { + delete rest.displayValue; + delete rest.onChange; + delete rest.value; + + return ( +
+ {Array.from({length: max}, (_, i) => { + const starValue = Math.min(Math.max(shownValue - i, 0), 1); + + return ( + 0} + value={starValue} + onClick={onStarClick} + /> + ); + })} +
+ ); + } }); const StarRatingsDecorator = compose( - Changeable, - SpotlightContainerDecorator({enterTo: 'last-focused'}), - Skinnable + Changeable, + SpotlightContainerDecorator({enterTo: 'last-focused'}), + Skinnable ); const StarRatings = StarRatingsDecorator(StarRatingsBase); export default StarRatings; export { - StarRatings, - StarRatingsBase, - StarRatingsDecorator -}; \ No newline at end of file + StarRatings, + StarRatingsBase, + StarRatingsDecorator +}; diff --git a/samples/sampler/stories/default/Star.js b/samples/sampler/stories/default/Star.js index 0c4812a37..5a8520945 100644 --- a/samples/sampler/stories/default/Star.js +++ b/samples/sampler/stories/default/Star.js @@ -6,32 +6,32 @@ import {boolean, range} from '@enact/storybook-utils/addons/controls'; const StarConfig = mergeComponentMetadata('Star', Star, StarBase); export default { - title: 'Limestone/StarRatings', - component: 'Star' + title: 'Limestone/StarRatings', + component: 'Star' }; export const _Star = (args) => { - const actions = { - onClick: action('onClick'), - }; + const actions = { + onClick: action('onClick') + }; - const controls = { - active: args['active'], - highlighted: args['highlighted'], - value: args['value'] - }; + const controls = { + active: args['active'], + highlighted: args['highlighted'], + value: args['value'] + }; - return (<> - - ); -} + return (<> + + ); +}; boolean('active', _Star, StarConfig); boolean('highlighted', _Star, StarConfig); range('value', _Star, StarConfig, {min: 0, max: 1, step: 0.01}); -_Star.storyName = 'Star'; \ No newline at end of file +_Star.storyName = 'Star'; diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index b2e21def5..6ef7b471e 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -6,30 +6,30 @@ import StarRatings, {StarRatingsBase} from '../../../../StarRatings/StarRatings' const StarRatingsConfig = mergeComponentMetadata('Star', StarRatings, StarRatingsBase); export default { - title: 'Limestone/StarRatings', - component: 'StarRatings' + title: 'Limestone/StarRatings', + component: 'StarRatings' }; export const _StarRatings = (args) => { - const actions = { - onChange: action('onChange') - }; + const actions = { + onChange: action('onChange') + }; - const controls = { - active: args['active'], - displayValue: args['displayValue'] - }; + const controls = { + active: args['active'], + displayValue: args['displayValue'] + }; - return (<> - - ); -} + return (<> + + ); +}; boolean('active', _StarRatings, StarRatingsConfig); range('displayValue', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}); -_StarRatings.storyName = 'StarRatings'; \ No newline at end of file +_StarRatings.storyName = 'StarRatings'; From 3f80316e53fe4a8b9a2fbd120fb5af171f841811 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:19:58 +0300 Subject: [PATCH 04/17] Update samples/sampler/stories/default/StarRatings.js Co-authored-by: Daniel Stoian <63335068+daniel-stoian-lgp@users.noreply.github.com> --- samples/sampler/stories/default/StarRatings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index 6ef7b471e..e042e4d73 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -3,7 +3,7 @@ import {action} from '@enact/storybook-utils/addons/actions'; import {boolean, range} from '@enact/storybook-utils/addons/controls'; import StarRatings, {StarRatingsBase} from '../../../../StarRatings/StarRatings'; -const StarRatingsConfig = mergeComponentMetadata('Star', StarRatings, StarRatingsBase); +const StarRatingsConfig = mergeComponentMetadata('StarRatings', StarRatings, StarRatingsBase); export default { title: 'Limestone/StarRatings', From 4ce8904b53ce1f454568f02929db2c2df650850a Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:52:20 +0300 Subject: [PATCH 05/17] Review fixes --- StarRatings/Star.js | 119 +++++-- StarRatings/Star.module.less | 9 +- StarRatings/StarRatings.js | 266 ++++++++++++---- StarRatings/StarRatings.module.less | 4 +- StarRatings/tests/Star-specs.js | 167 ++++++++++ StarRatings/tests/StarRatings-specs.js | 295 ++++++++++++++++++ samples/sampler/stories/default/Star.js | 2 +- .../sampler/stories/default/StarRatings.js | 20 +- 8 files changed, 778 insertions(+), 104 deletions(-) create mode 100644 StarRatings/tests/Star-specs.js create mode 100644 StarRatings/tests/StarRatings-specs.js diff --git a/StarRatings/Star.js b/StarRatings/Star.js index 6e31ae896..4c146d115 100644 --- a/StarRatings/Star.js +++ b/StarRatings/Star.js @@ -11,50 +11,82 @@ import css from './Star.module.less'; const SpottableIcon = Spottable(Icon); +/** + * A single star of a [StarRatings]{@link limestone/StarRatings.StarRatings}, without any + * behaviors applied. + * + * The star fills from the start of the reading direction (left in LTR, right in RTL) by the + * amount set in `value`. + * + * @class StarBase + * @memberof limestone/StarRatings + * @ui + * @private + */ const StarBase = kind({ name: 'Star', - propTypes: { + propTypes: /** @lends limestone/StarRatings.StarBase.prototype */ { /** - * Unique identifier for the star. - * - * @type {String} - * @required - * @public - */ + * Unique identifier for the star. + * + * Also sent to `onClick`, so the parent can tell which star was selected. + * + * @type {String} + * @required + * @public + */ id: PropTypes.string.isRequired, /** - * When `true`, the star is focusable and clickable. - * - * @type {Boolean} - * @default false - * @public - */ - active: PropTypes.bool, + * Makes the star glow while any star in its row has focus. + * + * @type {Boolean} + * @default false + * @public + */ + highlighted: PropTypes.bool, /** - * When `true`, the star glows while its row has focus. - * - * @type {Boolean} - * @default false - * @public - */ - highlighted: PropTypes.bool, + * Called when the star is selected. + * + * Receives an object with the star's `id` and the original event as `ev`. + * Not called while `readOnly` is `true`. + * + * @type {Function} + * @public + */ + onClick: PropTypes.func, + + /** + * Makes the star display-only. + * + * When `false`, the star can be focused with Spotlight and `onClick` is called when it is + * clicked, tapped or selected with the remote. When `true`, the star cannot be focused + * and ignores input. + * + * @type {Boolean} + * @default false + * @public + */ + readOnly: PropTypes.bool, /** - * Fill amount from 0 to 1, rounded to 10% steps. - * - * @type {Number} - * @default 0 - * @public - */ + * The fill amount, from `0` (empty) to `1` (full). + * + * Rounded half-up to 10% steps, e.g. `0.55` fills 60%. Values outside `0`–`1` are clamped, + * and `NaN` shows an empty star. + * + * @type {Number} + * @default 0 + * @public + */ value: PropTypes.number }, defaultProps: { - active: false, highlighted: false, + readOnly: false, value: 0 }, @@ -65,7 +97,7 @@ const StarBase = kind({ handlers: { onClick: handle( - forProp('active', true), + forProp('readOnly', false), adaptEvent( (ev, {id}) => ({id, ev}), forward('onClick') @@ -76,16 +108,17 @@ const StarBase = kind({ computed: { className: ({highlighted, styler}) => styler.append({highlighted}), style: ({style, value}) => { - const percent = Math.round(value * 10) * 10; - return {...style, '--star-fill': percent + '%'}; + // Whole tenths, clamped to 0–10; `|| 0` also turns NaN into an empty star + const tenths = Math.min(Math.max(Math.round(value * 10) || 0, 0), 10); + return {...style, '--star-fill': tenths * 10 + '%'}; } }, - render: ({active, className, id, onClick, style, ...rest}) => { + render: ({className, id, onClick, readOnly, style, ...rest}) => { delete rest.highlighted; delete rest.value; - const IconComponent = active ? SpottableIcon : Icon; + const IconComponent = readOnly ? Icon : SpottableIcon; return ( .star.highlighted { diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index 901d764b6..8bdc72cc3 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -1,9 +1,31 @@ +/** + * Limestone styled star rating components and behaviors. + * + * A row of stars that either displays a rating or lets the user choose one. + * + * @example + * + * + * @example + * console.log(value)} + * /> + * + * @module limestone/StarRatings + * @exports StarRatings + * @exports StarRatingsBase + * @exports StarRatingsDecorator + */ + import {adaptEvent, forProp, forward, handle} from '@enact/core/handle'; import kind from '@enact/core/kind'; import Spotlight from '@enact/spotlight'; import SpotlightContainerDecorator from '@enact/spotlight/SpotlightContainerDecorator'; import Changeable from '@enact/ui/Changeable'; import PropTypes from 'prop-types'; +import clamp from 'ramda/src/clamp'; import compose from 'ramda/src/compose'; import Skinnable from '../Skinnable'; @@ -12,7 +34,14 @@ import Star from './Star'; import css from './StarRatings.module.less'; -// Returns the star element under the current touch point, or null +/** + * Finds the star element under the current touch point. + * + * @param {TouchEvent} ev The touch event. + * @param {String} id The `id` of the `StarRatings` the star must belong to. + * @returns {Element|null} The star element, or `null` if the touch is not over a star. + * @private + */ const getStarFromTouch = (ev, id) => { const touch = ev.touches && ev.touches[0]; if (!touch) return null; @@ -21,10 +50,26 @@ const getStarFromTouch = (ev, id) => { return el && el.closest(`[id^="${id}-star-"]`); }; -// Reads the star number (1-based) from a star id like `${id}-star-3` +/** + * Reads the 1-based star number from a star id, e.g. `3` from `rating-star-3`. + * + * @param {String} starId The star's id. + * @param {String} id The `id` of its `StarRatings`. + * @returns {Number} The star number. + * @private + */ const getStarNumber = (starId, id) => Number(starId.slice(`${id}-star-`.length)); -// Moves focus to the star, which triggers the :focus-within glow +/** + * Moves focus to a star, which shows the focus glow on the row. + * + * Uses Spotlight so remote navigation continues from this star, and falls back to native focus + * if Spotlight declines. + * + * @param {Element|null} star The star element to focus. + * @returns {undefined} + * @private + */ const focusStar = (star) => { if (star && document.activeElement !== star) { if (!Spotlight.focus(star)) { @@ -33,69 +78,127 @@ const focusStar = (star) => { } }; +/** + * The rating shown while selecting when no `value` or `defaultValue` is given. The spec + * recommends 3 as the default selection. + * + * @type {Number} + * @private + */ +const DEFAULT_SELECTING_VALUE = 3; + +/** + * Converts a rating to whole tenths of a star, the unit the stars are filled in. + * + * * Read-only: rounded half-up to one decimal place and clamped to `0`–`max`, so `2.54` gives + * `25` and `2.55` gives `26`. Rounding once, then working in integers, avoids floating-point + * subtraction errors (`2.55 - 2 = 0.5499…`). + * * Selecting: rounded half-up to a whole star and clamped to `1`–`max`, the selectable range. + * A missing value or `NaN` uses `DEFAULT_SELECTING_VALUE` (3, also clamped to `max`). + * + * @param {Number} value The rating. + * @param {Number} max The number of stars. + * @param {Boolean} readOnly Whether the component is read-only. + * @returns {Number} The rating in tenths, from `0` to `max * 10`. + * @private + */ +const toTenths = (value, max, readOnly) => { + if (readOnly) { + return clamp(0, max * 10, Math.round(value * 10) || 0); + } + + const hasValue = typeof value === 'number' && !Number.isNaN(value); + const rating = hasValue ? value : DEFAULT_SELECTING_VALUE; + + return clamp(1, max, Math.round(rating)) * 10; +}; + +/** + * A star rating component, without any behaviors applied. + * + * Has two modes, set by `readOnly`: + * * Read-only (`readOnly` is `true`): displays `value`, rounded half-up to one decimal place, so + * `2.54` fills 2.5 stars and `2.55` fills 2.6 stars. Not focusable and ignores all input. + * * Selecting (`readOnly` is `false`, the default): shows `value` as whole stars, from 1 to + * `max`, starting at 3 stars when no value is given. The user selects a star with the + * remote, a click, a tap or a touch drag, and `onChange` is called with the star's number. + * + * @class StarRatingsBase + * @memberof limestone/StarRatings + * @ui + * @public + */ const StarRatingsBase = kind({ name: 'StarRatings', - propTypes: { + propTypes: /** @lends limestone/StarRatings.StarRatingsBase.prototype */ { /** - * Unique identifier, used to build each star's id. - * - * @type {String} - * @required - * @public - */ + * Unique identifier for the component. + * + * Each star gets the id `{id}-star-{number}`, e.g. `rating-star-3`, so the id must be + * unique on the page and must not contain quotes. + * + * @type {String} + * @required + * @public + */ id: PropTypes.string.isRequired, /** - * `false` = display mode (shows `displayValue`, not interactive) - * `true` = edit mode (shows `value`, interactive) - * - * @type {Boolean} - * @default false - * @public - */ - active: PropTypes.bool, - - /** - * Value shown in display mode. Supports fractions, e.g. 3.7. - * - * @type {Number} - * @default 0 - * @public - */ - displayValue: PropTypes.number, - - /** - * Number of stars. - * - * @type {Number} - * @default 5 - * @public - */ + * The number of stars. + * + * @type {Number} + * @default 5 + * @public + */ max: PropTypes.number, /** - * Called in edit mode with `{value}`: the number of the selected star. - * - * @type {Function} - * @public - */ + * Called when the user selects a star. + * + * Receives an object whose `value` is the number of the selected star, from `1` to `max`. + * During a touch drag, it is called whenever the finger moves onto a star other than the + * current `value`. Never called while `readOnly` is `true`. + * + * @type {Function} + * @public + */ onChange: PropTypes.func, /** - * Value shown in edit mode. If omitted, the component keeps its own - * internal value (via Changeable). Use `defaultValue` to set a starting value. - * - * @type {Number} - * @public - */ + * Displays the rating without letting the user change it. + * + * * `true`: the stars cannot be focused or selected, and `value` keeps one decimal place. + * * `false`: the user can select a star, `value` is shown as whole stars, and the filled + * stars glow while the row has focus. + * + * @type {Boolean} + * @default false + * @public + */ + readOnly: PropTypes.bool, + + /** + * The rating. + * + * * Read-only: rounded half-up to one decimal place and clamped to `0`–`max`. When not + * set or `NaN`, no star is filled. + * * Selecting: rounded half-up to a whole star and clamped to `1`–`max`, so `0` shows 1 + * star. When neither `value` nor `defaultValue` is set (or the value is `NaN`), shows + * 3 stars, the spec's recommended default. + * + * If not set, the component keeps its own value and updates it on each selection; use + * `defaultValue` to set where it starts. + * + * @type {Number} + * @public + */ value: PropTypes.number }, defaultProps: { - active: false, - displayValue: 0, - max: 5 + max: 5, + readOnly: false }, styles: { @@ -106,7 +209,7 @@ const StarRatingsBase = kind({ handlers: { // Star forwards {id, ev}; turn it into {value} for onChange onStarClick: handle( - forProp('active', true), + forProp('readOnly', false), adaptEvent( ({id: starId}, {id}) => ({value: getStarNumber(starId, id)}), forward('onChange') @@ -116,7 +219,7 @@ const StarRatingsBase = kind({ // Glow starts as soon as the finger touches a star onTouchStart: handle( forward('onTouchStart'), - forProp('active', true), + forProp('readOnly', false), (ev, {id}) => { focusStar(getStarFromTouch(ev, id)); return true; @@ -126,7 +229,7 @@ const StarRatingsBase = kind({ // Drag across the stars to change the rating onTouchMove: handle( forward('onTouchMove'), - forProp('active', true), + forProp('readOnly', false), (ev, {id, onChange, value}) => { const star = getStarFromTouch(ev, id); if (!star) return false; // outside the row: keep the last value and focus @@ -143,12 +246,11 @@ const StarRatingsBase = kind({ }, computed: { - className: ({active, styler}) => styler.append({active}), - shownValue: ({active, displayValue, value}) => (active ? value : displayValue) || 0 + className: ({readOnly, styler}) => styler.append({readOnly}), + tenths: ({max, readOnly, value}) => toTenths(value, max, readOnly) }, - render: ({active, id, max, onStarClick, onTouchMove, onTouchStart, shownValue, ...rest}) => { - delete rest.displayValue; + render: ({id, max, onStarClick, onTouchMove, onTouchStart, readOnly, tenths, ...rest}) => { delete rest.onChange; delete rest.value; @@ -160,14 +262,14 @@ const StarRatingsBase = kind({ onTouchStart={onTouchStart} > {Array.from({length: max}, (_, i) => { - const starValue = Math.min(Math.max(shownValue - i, 0), 1); + const starValue = clamp(0, 10, tenths - i * 10) / 10; return ( 0} + highlighted={!readOnly && starValue > 0} + readOnly={readOnly} value={starValue} onClick={onStarClick} /> @@ -178,14 +280,60 @@ const StarRatingsBase = kind({ } }); +/** + * Applies Limestone specific behaviors to + * [StarRatingsBase]{@link limestone/StarRatings.StarRatingsBase} components. + * + * Stores the rating itself when `value` is not set (see `defaultValue`), groups the stars for + * Spotlight navigation (focus returns to the last focused star) and applies the current skin. + * + * @hoc + * @memberof limestone/StarRatings + * @mixes ui/Changeable.Changeable + * @mixes spotlight/SpotlightContainerDecorator.SpotlightContainerDecorator + * @mixes limestone/Skinnable.Skinnable + * @public + */ const StarRatingsDecorator = compose( Changeable, SpotlightContainerDecorator({enterTo: 'last-focused'}), Skinnable ); +/** + * A Limestone styled star rating component, with Spotlight, touch and RTL support. + * + * Usage: + * ``` + * // Read-only, e.g. an app's average rating + * + * + * // Selecting, controlled + * setRating(value)} /> + * ``` + * + * @class StarRatings + * @memberof limestone/StarRatings + * @extends limestone/StarRatings.StarRatingsBase + * @mixes limestone/StarRatings.StarRatingsDecorator + * @ui + * @public + */ const StarRatings = StarRatingsDecorator(StarRatingsBase); +/** + * The rating to start from when `value` is not set. + * + * Works in both modes and is rounded and clamped like `value`. After that, the component + * updates the rating itself on each selection. When not set, selecting starts at 3 stars and + * read-only shows no stars. + * + * @name defaultValue + * @memberof limestone/StarRatings.StarRatings.prototype + * @type {Number} + * @public + */ + export default StarRatings; export { StarRatings, diff --git a/StarRatings/StarRatings.module.less b/StarRatings/StarRatings.module.less index f785f2e1e..14236caca 100644 --- a/StarRatings/StarRatings.module.less +++ b/StarRatings/StarRatings.module.less @@ -4,9 +4,9 @@ -webkit-user-select: none; user-select: none; - // In edit mode, horizontal finger movement drives the rating + // While selecting, horizontal finger movement drives the rating // instead of scrolling; vertical page scrolling still works - &.active { + &:not(.readOnly) { touch-action: pan-y; } diff --git a/StarRatings/tests/Star-specs.js b/StarRatings/tests/Star-specs.js new file mode 100644 index 000000000..7592eb90d --- /dev/null +++ b/StarRatings/tests/Star-specs.js @@ -0,0 +1,167 @@ +import '@testing-library/jest-dom'; +import {fireEvent, render} from '@testing-library/react'; + +import Icon from '../../Icon'; +import Star from '../Star'; + +const ID = 'star'; + +const getStar = () => document.getElementById(ID); + +const getFill = () => getStar().style.getPropertyValue('--star-fill'); + +describe('Star', () => { + describe('rendering', () => { + // Icon maps the name `star` to a font glyph, so compare with what Icon renders + test('should render the `star` icon', () => { + const {container} = render(star); + const expectedGlyph = container.firstChild.textContent; + + render(); + + expect(getStar().textContent).not.toBe(''); + expect(getStar().textContent).toBe(expectedGlyph); + }); + + test('should apply `id` to the root element', () => { + render(); + + expect(getStar()).toBeInTheDocument(); + }); + + test('should have the `star` class', () => { + render(); + + expect(getStar()).toHaveClass('star'); + }); + + test('should keep a custom `className`', () => { + render(); + + expect(getStar()).toHaveClass('star'); + expect(getStar()).toHaveClass('custom'); + }); + + test('should not pass `value` or `highlighted` to the DOM', () => { + render(); + + expect(getStar()).not.toHaveAttribute('value'); + expect(getStar()).not.toHaveAttribute('highlighted'); + }); + }); + + describe('fill (`value` → --star-fill)', () => { + test('should be empty by default', () => { + render(); + + expect(getFill()).toBe('0%'); + }); + + test.each([ + [0, '0%'], + [0.1, '10%'], + [0.5, '50%'], + [0.7, '70%'], + [1, '100%'] + ])('should fill %p as %s', (value, expected) => { + render(); + + expect(getFill()).toBe(expected); + }); + + test.each([ + [0.04, '0%'], + [0.05, '10%'], + [0.54, '50%'], + [0.55, '60%'], + [0.94, '90%'], + [0.95, '100%'] + ])('should round %p half-up to %s', (value, expected) => { + render(); + + expect(getFill()).toBe(expected); + }); + + test.each([ + [-0.5, '0%'], + [-10, '0%'], + [1.5, '100%'], + [10, '100%'] + ])('should clamp %p to %s', (value, expected) => { + render(); + + expect(getFill()).toBe(expected); + }); + + test('should show an empty star for NaN', () => { + render(); + + expect(getFill()).toBe('0%'); + }); + + test('should keep other inline styles', () => { + render(); + + expect(getStar().style.color).toBe('red'); + expect(getFill()).toBe('50%'); + }); + }); + + describe('`highlighted`', () => { + test('should not have the `highlighted` class by default', () => { + render(); + + expect(getStar()).not.toHaveClass('highlighted'); + }); + + test('should add the `highlighted` class when true', () => { + render(); + + expect(getStar()).toHaveClass('highlighted'); + }); + }); + + describe('`readOnly`', () => { + test('should be focusable by default', () => { + render(); + + expect(getStar()).toHaveClass('spottable'); + }); + + test('should not be focusable when true', () => { + render(); + + expect(getStar()).not.toHaveClass('spottable'); + }); + }); + + describe('`onClick`', () => { + test('should be called with `{id, ev}`', () => { + const handleClick = jest.fn(); + render(); + + fireEvent.click(getStar()); + + expect(handleClick).toHaveBeenCalledTimes(1); + expect(handleClick).toHaveBeenCalledWith(expect.objectContaining({ + id: ID, + ev: expect.objectContaining({type: 'click'}) + })); + }); + + test('should not be called when read-only', () => { + const handleClick = jest.fn(); + render(); + + fireEvent.click(getStar()); + + expect(handleClick).not.toHaveBeenCalled(); + }); + + test('should not throw on click when no handler is given', () => { + render(); + + expect(() => fireEvent.click(getStar())).not.toThrow(); + }); + }); +}); diff --git a/StarRatings/tests/StarRatings-specs.js b/StarRatings/tests/StarRatings-specs.js new file mode 100644 index 000000000..1a3ff302e --- /dev/null +++ b/StarRatings/tests/StarRatings-specs.js @@ -0,0 +1,295 @@ +import '@testing-library/jest-dom'; +import {fireEvent, render} from '@testing-library/react'; + +import StarRatings from '../StarRatings'; + +const ID = 'rating'; + +const getStar = (number) => document.getElementById(`${ID}-star-${number}`); + +const getStars = () => Array.from(document.querySelectorAll(`[id^="${ID}-star-"]`)); + +// Fill of each star as rendered, e.g. ['100%', '100%', '50%', '0%', '0%'] +const getFills = () => getStars().map((star) => star.style.getPropertyValue('--star-fill')); + +// Total number of stars filled, e.g. 2.5 (sums whole percents to avoid float error) +const getFilledTotal = () => getFills().reduce((sum, fill) => sum + parseInt(fill), 0) / 100; + +describe('StarRatings', () => { + describe('read-only rounding (spec: half-up to one decimal place)', () => { + // Regression: per-star subtraction made 2.55 - 2 = 0.5499…, which rounded down to 50% + test('should fill 2.5 stars for 2.54', () => { + render(); + + expect(getFills()).toEqual(['100%', '100%', '50%', '0%', '0%']); + expect(getFilledTotal()).toBe(2.5); + }); + + test('should fill 2.6 stars for 2.55', () => { + render(); + + expect(getFills()).toEqual(['100%', '100%', '60%', '0%', '0%']); + expect(getFilledTotal()).toBe(2.6); + }); + + test.each([ + [0.04, 0], + [0.05, 0.1], + [1.15, 1.2], + [1.45, 1.5], + [3.3, 3.3], + [3.75, 3.8], + [4.94, 4.9], + [4.95, 5] + ])('should fill %p as %p stars', (value, expected) => { + render(); + + expect(getFilledTotal()).toBe(expected); + }); + + test.each([ + [-1, 0], + [0, 0], + [7, 5], + [null, 0], + [NaN, 0] + ])('should clamp %p to %p stars', (value, expected) => { + render(); + + expect(getFilledTotal()).toBe(expected); + }); + + test('should show no stars when `value` is not set', () => { + render(); + + expect(getFilledTotal()).toBe(0); + }); + }); + + describe('selecting rounding (whole stars, 1 to max)', () => { + test.each([ + [3, 3], + [2.4, 2], + [2.5, 3], + [3.3, 3], + [0.3, 1], + [7, 5] + ])('should show %p as %p stars', (value, expected) => { + render(); + + expect(getFilledTotal()).toBe(expected); + }); + + // Spec: the lowest selectable value is 1 + test.each([ + [0], + [-1] + ])('should clamp %p up to 1 star', (value) => { + render(); + + expect(getFilledTotal()).toBe(1); + }); + + // Spec: 3 is the recommended default selection + test('should show 3 stars when neither `value` nor `defaultValue` is set', () => { + render(); + + expect(getFilledTotal()).toBe(3); + }); + + test.each([ + [null], + [NaN] + ])('should show the default 3 stars for %p', (value) => { + render(); + + expect(getFilledTotal()).toBe(3); + }); + + test('should clamp the default to `max`', () => { + render(); + + expect(getFilledTotal()).toBe(2); + }); + + test('should let the user change the default selection', () => { + const handleChange = jest.fn(); + render(); + + fireEvent.click(getStar(5)); + + expect(handleChange).toHaveBeenCalledWith(expect.objectContaining({value: 5})); + expect(getFilledTotal()).toBe(5); + }); + + test('should start from `defaultValue` when `value` is not set', () => { + render(); + + expect(getFilledTotal()).toBe(2); + }); + }); + + describe('switching modes', () => { + test('should show the same `value` in both modes', () => { + const {rerender} = render(); + expect(getFilledTotal()).toBe(3.3); + + rerender(); + expect(getFilledTotal()).toBe(3); + }); + }); + + describe('rendering', () => { + test('should render 5 stars by default', () => { + render(); + + expect(getStars()).toHaveLength(5); + }); + + test('should render `max` stars', () => { + render(); + + expect(getStars()).toHaveLength(10); + expect(getFilledTotal()).toBe(7.5); + }); + + test('should add the `readOnly` class when read-only', () => { + render(); + + expect(document.getElementById(ID)).toHaveClass('readOnly'); + }); + + test('should not add the `readOnly` class when selecting', () => { + render(); + + expect(document.getElementById(ID)).not.toHaveClass('readOnly'); + }); + }); + + describe('read-only mode', () => { + test('should not make stars focusable', () => { + render(); + + getStars().forEach((star) => expect(star).not.toHaveClass('spottable')); + }); + + test('should not highlight any star', () => { + render(); + + getStars().forEach((star) => expect(star).not.toHaveClass('highlighted')); + }); + + test('should not call `onChange` or change the value on click', () => { + const handleChange = jest.fn(); + render(); + + fireEvent.click(getStar(4)); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getFilledTotal()).toBe(3); + }); + }); + + describe('selecting mode', () => { + test('should make stars focusable', () => { + render(); + + getStars().forEach((star) => expect(star).toHaveClass('spottable')); + }); + + test('should highlight only filled stars', () => { + render(); + + expect(getStar(1)).toHaveClass('highlighted'); + expect(getStar(2)).toHaveClass('highlighted'); + expect(getStar(3)).toHaveClass('highlighted'); + expect(getStar(4)).not.toHaveClass('highlighted'); + expect(getStar(5)).not.toHaveClass('highlighted'); + }); + + test('should call `onChange` with the number of the clicked star', () => { + const handleChange = jest.fn(); + render(); + + fireEvent.click(getStar(3)); + + expect(handleChange).toHaveBeenCalledTimes(1); + expect(handleChange).toHaveBeenCalledWith(expect.objectContaining({value: 3})); + }); + + test('should remember the clicked star when `value` is not set', () => { + render(); + + fireEvent.click(getStar(4)); + + expect(getFilledTotal()).toBe(4); + }); + + test('should keep showing a controlled `value` until the parent updates it', () => { + const handleChange = jest.fn(); + render(); + + fireEvent.click(getStar(4)); + + expect(handleChange).toHaveBeenCalledWith(expect.objectContaining({value: 4})); + expect(getFilledTotal()).toBe(2); + }); + }); + + describe('touch drag', () => { + const originalElementFromPoint = document.elementFromPoint; + + afterEach(() => { + document.elementFromPoint = originalElementFromPoint; + }); + + // jsdom has no layout, so point the "finger" at a given star + const touchMoveOver = (star) => { + document.elementFromPoint = jest.fn(() => star); + fireEvent.touchMove(document.getElementById(ID), { + touches: [{clientX: 0, clientY: 0}] + }); + }; + + test('should call `onChange` with the star under the finger', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(4)); + + expect(handleChange).toHaveBeenCalledWith(expect.objectContaining({value: 4})); + expect(getFilledTotal()).toBe(4); + }); + + test('should not call `onChange` again while staying on the same star', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(2)); + touchMoveOver(getStar(2)); + + expect(handleChange).toHaveBeenCalledTimes(1); + }); + + test('should keep the last value when the finger leaves the row', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(3)); + touchMoveOver(document.body); + + expect(handleChange).toHaveBeenCalledTimes(1); + expect(getFilledTotal()).toBe(3); + }); + + test('should not call `onChange` when read-only', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(4)); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getFilledTotal()).toBe(1); + }); + }); +}); diff --git a/samples/sampler/stories/default/Star.js b/samples/sampler/stories/default/Star.js index 5a8520945..52b10ce41 100644 --- a/samples/sampler/stories/default/Star.js +++ b/samples/sampler/stories/default/Star.js @@ -6,7 +6,7 @@ import {boolean, range} from '@enact/storybook-utils/addons/controls'; const StarConfig = mergeComponentMetadata('Star', Star, StarBase); export default { - title: 'Limestone/StarRatings', + title: 'Limestone/Star', component: 'Star' }; diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index e042e4d73..73c7b0a47 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -1,6 +1,7 @@ import {mergeComponentMetadata} from '@enact/storybook-utils'; import {action} from '@enact/storybook-utils/addons/actions'; import {boolean, range} from '@enact/storybook-utils/addons/controls'; +import {useCallback, useState} from 'react'; import StarRatings, {StarRatingsBase} from '../../../../StarRatings/StarRatings'; const StarRatingsConfig = mergeComponentMetadata('StarRatings', StarRatings, StarRatingsBase); @@ -11,25 +12,28 @@ export default { }; export const _StarRatings = (args) => { - const actions = { - onChange: action('onChange') - }; + const [defaultValue, setDefaultValue] = useState(0); + + const handleValue = useCallback((event) => { + action('onChange'); + setDefaultValue(event.value); + }, []); const controls = { - active: args['active'], - displayValue: args['displayValue'] + value: args['readOnly'] ? args['value'] : defaultValue, + readOnly: args['readOnly'] }; return (<> ); }; -boolean('active', _StarRatings, StarRatingsConfig); -range('displayValue', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}); +boolean('readOnly', _StarRatings, StarRatingsConfig); +range('value', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}); _StarRatings.storyName = 'StarRatings'; From f5d0ea1ab1f595f191b4682c8fe02b7ed2a3dbb4 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:06:01 +0300 Subject: [PATCH 06/17] Lint fixes --- samples/sampler/stories/default/StarRatings.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index 73c7b0a47..a28293f1b 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -12,12 +12,12 @@ export default { }; export const _StarRatings = (args) => { - const [defaultValue, setDefaultValue] = useState(0); + const [defaultValue, setDefaultValue] = useState(0); // eslint-disable-line react-hooks/rules-of-hooks const handleValue = useCallback((event) => { action('onChange'); setDefaultValue(event.value); - }, []); + }, []); // eslint-disable-line react-hooks/rules-of-hooks const controls = { value: args['readOnly'] ? args['value'] : defaultValue, From 3662703db6c43d025988322946fdd403a4530146 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:15:18 +0300 Subject: [PATCH 07/17] Lint fixes --- samples/sampler/stories/default/StarRatings.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index a28293f1b..ec2d6184a 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -14,10 +14,10 @@ export default { export const _StarRatings = (args) => { const [defaultValue, setDefaultValue] = useState(0); // eslint-disable-line react-hooks/rules-of-hooks - const handleValue = useCallback((event) => { + const handleValue = useCallback((event) => { // eslint-disable-line react-hooks/rules-of-hooks action('onChange'); setDefaultValue(event.value); - }, []); // eslint-disable-line react-hooks/rules-of-hooks + }, []); const controls = { value: args['readOnly'] ? args['value'] : defaultValue, From 13b65c6567a7afaaca72f843bde13f5661df73d9 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Tue, 20 Oct 2026 15:12:51 +0300 Subject: [PATCH 08/17] Applied GUI --- StarRatings/Star.js | 67 +++++++-- StarRatings/Star.module.less | 139 ++++++++++++++++-- StarRatings/StarRatings.js | 73 +++++++-- StarRatings/StarRatings.module.less | 15 +- StarRatings/tests/Star-specs.js | 96 +++++++++--- StarRatings/tests/StarRatings-specs.js | 132 ++++++++++++++--- samples/sampler/stories/default/Star.js | 8 +- .../sampler/stories/default/StarRatings.js | 10 +- 8 files changed, 451 insertions(+), 89 deletions(-) diff --git a/StarRatings/Star.js b/StarRatings/Star.js index 4c146d115..9f8e42726 100644 --- a/StarRatings/Star.js +++ b/StarRatings/Star.js @@ -5,18 +5,37 @@ import PropTypes from 'prop-types'; import compose from 'ramda/src/compose'; import Icon from '../Icon'; +import iconList from '../Icon/IconList'; import Skinnable from '../Skinnable'; import css from './Star.module.less'; const SpottableIcon = Spottable(Icon); +// The star glyph, also drawn by the fill layer (`::after`) on top of the focus outline +const STAR_GLYPH = String.fromCodePoint(iconList.star); + +/** + * Where the fill edge sits (`--star-fill`, % of the star's box) for each tenth of `value`. + * + * The glyph only covers about 9%–91% of its box, so plain tenths look wrong: a 10% fill shows + * almost nothing and a 90% fill looks full. 0.1 and 0.9 are pinned to 20% and 80%; the steps in + * between were measured on the Limestone star glyph so each tenth adds an equal share of the + * star's area (symmetric around 50%). + * + * @type {Number[]} + * @private + */ +const FILL_STEPS = [0, 20, 33, 39, 45, 50, 55, 61, 67, 80, 100]; + /** * A single star of a [StarRatings]{@link limestone/StarRatings.StarRatings}, without any * behaviors applied. * * The star fills from the start of the reading direction (left in LTR, right in RTL) by the - * amount set in `value`. + * amount set in `value`. While focused, it shows a white outline and a drop shadow that follow + * the star's shape. The star is a 108px Icon (Limestone has no preset that size, so the number + * is passed to `size`). * * @class StarBase * @memberof limestone/StarRatings @@ -39,25 +58,41 @@ const StarBase = kind({ id: PropTypes.string.isRequired, /** - * Makes the star glow while any star in its row has focus. + * Disables the star and makes it non-interactive. + * + * The star is shown at 30% opacity and `onClick` is not called. Like other disabled + * Limestone controls, it can still receive focus with the remote unless `readOnly` is also + * set. * * @type {Boolean} * @default false * @public */ - highlighted: PropTypes.bool, + disabled: PropTypes.bool, /** * Called when the star is selected. * * Receives an object with the star's `id` and the original event as `ev`. - * Not called while `readOnly` is `true`. + * Not called while `disabled` or `readOnly` is `true`. * * @type {Function} * @public */ onClick: PropTypes.func, + /** + * Colors the empty (unselected) part of the star for the surface behind it. + * + * * `false`: `#FFFFFF33` (white at 20%), for dark backgrounds. + * * `true`: `#1111111A` (near-black at 10%), for light overlay surfaces. + * + * @type {Boolean} + * @default false + * @public + */ + overlayMode: PropTypes.bool, + /** * Makes the star display-only. * @@ -74,8 +109,8 @@ const StarBase = kind({ /** * The fill amount, from `0` (empty) to `1` (full). * - * Rounded half-up to 10% steps, e.g. `0.55` fills 60%. Values outside `0`–`1` are clamped, - * and `NaN` shows an empty star. + * Rounded half-up to tenths (`0.55` → `0.6`), and each tenth fills the star by an equal + * share of its area. Values outside `0`–`1` are clamped, and `NaN` shows an empty star. * * @type {Number} * @default 0 @@ -85,7 +120,8 @@ const StarBase = kind({ }, defaultProps: { - highlighted: false, + disabled: false, + overlayMode: false, readOnly: false, value: 0 }, @@ -97,6 +133,7 @@ const StarBase = kind({ handlers: { onClick: handle( + forProp('disabled', false), forProp('readOnly', false), adaptEvent( (ev, {id}) => ({id, ev}), @@ -106,16 +143,16 @@ const StarBase = kind({ }, computed: { - className: ({highlighted, styler}) => styler.append({highlighted}), + className: ({overlayMode, styler}) => styler.append({overlayMode}), style: ({style, value}) => { // Whole tenths, clamped to 0–10; `|| 0` also turns NaN into an empty star const tenths = Math.min(Math.max(Math.round(value * 10) || 0, 0), 10); - return {...style, '--star-fill': tenths * 10 + '%'}; + return {...style, '--star-fill': FILL_STEPS[tenths] + '%'}; } }, - render: ({className, id, onClick, readOnly, style, ...rest}) => { - delete rest.highlighted; + render: ({className, disabled, id, onClick, readOnly, style, ...rest}) => { + delete rest.overlayMode; delete rest.value; const IconComponent = readOnly ? Icon : SpottableIcon; @@ -123,11 +160,14 @@ const StarBase = kind({ return ( star @@ -163,7 +203,8 @@ const Star = StarDecorator(StarBase); export default Star; export { + FILL_STEPS, Star, StarBase, StarDecorator -}; +}; \ No newline at end of file diff --git a/StarRatings/Star.module.less b/StarRatings/Star.module.less index 36cd4e14d..c61e27d59 100644 --- a/StarRatings/Star.module.less +++ b/StarRatings/Star.module.less @@ -2,8 +2,67 @@ @import "../styles/variables.less"; @import "../styles/skin.less"; +@star-active-color: #FFCA28; // filled part of a star +@star-inactive-color: #FFFFFF33; // empty part of a star (white at 20%) +@star-inactive-color-overlay: #1111111A; // empty part with `overlayMode` (near-black at 10%) + +// Focus sizes are in `em`, relative to the star's size (Icon sets font-size to the icon size), so +// they keep the mockup's proportions at any star size and resolution. They were measured on the +// focus mockup with the star at 180px: `(17 * @star-px)` is 17px on a 180px star, ~10px on 108px. +@star-px: (1em / 180); + +// Focus: white outline fully outside the star, and a soft shadow (matched to the design mockup) +@star-focus-outline-color: #FFFFFF; +@star-focus-outline-width: (17 * @star-px); +@star-focus-shadow: drop-shadow(0 (24 * @star-px) (10 * @star-px) rgba(0, 0, 0, 0.3)); + +// Room around the glyph for the outline layer, so its mask does not cut off the outline or shadow +@star-focus-layer-space: (40 * @star-px); +@star-focus-layer-space-bottom: (72 * @star-px); // the shadow falls downwards + +// The outline layer draws its own glyph in this key color; the knockout filter below turns every +// pixel of that color transparent, leaving only the outline outside the star (and its shadow). +@star-knockout-key: #FF0000; + +// SVG filter: red key pixels become transparent; white (outline) and black (shadow) are kept. +// Color matrix: R' = G' = B' = R, A' = A - R + G. It expects a white outline color. +// (A filter is used instead of a CSS mask because `-webkit-mask-clip: text` does not survive the +// CSS re-serialization Storybook's pseudo-states addon does; a `url()` filter does.) +@star-knockout-filter: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg'%3E%3Cfilter id='knockout' color-interpolation-filters='sRGB'%3E%3CfeColorMatrix type='matrix' values='1 0 0 0 0 1 0 0 0 0 1 0 0 0 0 -1 1 0 1 0'/%3E%3C/filter%3E%3C/svg%3E#knockout"); + +// A round outline made of 16 copies of the glyph, moved @w in every direction around a circle +.star-outline-shadows(@w; @color) { + text-shadow: + @w 0 0 @color, + (@w * 0.9239) (@w * 0.3827) 0 @color, + (@w * 0.7071) (@w * 0.7071) 0 @color, + (@w * 0.3827) (@w * 0.9239) 0 @color, + 0 @w 0 @color, + (@w * -0.3827) (@w * 0.9239) 0 @color, + (@w * -0.7071) (@w * 0.7071) 0 @color, + (@w * -0.9239) (@w * 0.3827) 0 @color, + -@w 0 0 @color, + (@w * -0.9239) (@w * -0.3827) 0 @color, + (@w * -0.7071) (@w * -0.7071) 0 @color, + (@w * -0.3827) (@w * -0.9239) 0 @color, + 0 -@w 0 @color, + (@w * 0.3827) (@w * -0.9239) 0 @color, + (@w * 0.7071) (@w * -0.7071) 0 @color, + (@w * 0.9239) (@w * -0.3827) 0 @color; +} + +// How a star is drawn: +// - the element's own glyph is transparent (it only sets the size and position); +// - `::after` draws the fill on top: the same glyph, colored by the fill gradient; +// - while focused, `::before` draws the outline and its shadow behind the fill. A filter cuts the +// glyph's own shape out of that layer, so the fill's colors look the same with or without focus. .star { - transition: filter 200ms ease-out; + // Empty-part color, switched by the `overlayMode` prop + --star-inactive-color: @star-inactive-color; + + &.overlayMode { + --star-inactive-color: @star-inactive-color-overlay; + } // Touch handling touch-action: manipulation; // no double-tap-zoom delay @@ -12,16 +71,72 @@ user-select: none; -webkit-touch-callout: none; // no long-press context menu + // Inside applySkins so these beat Icon's own `.icon` rules regardless of CSS load order .applySkins({ - // Partial stars fill from the reading direction's start (flipped for RTL below) - background: linear-gradient( - var(--star-direction, to right), - var(--semantic-color-on-surface-main) var(--star-fill, 0%), - var(--semantic-color-surface-toggle) var(--star-fill, 0%) - ); - -webkit-background-clip: text; - background-clip: text; + // Design: a 108px box (size={108} in Star.js) with 4px/2px padding inside it. + // The visible layers are positioned on the padding box, so the glyph stays centered. + box-sizing: border-box; + // 2px is written in rem: Enact pins px values this small to a fixed 1px at every resolution + padding: 4px (2rem / 48); + margin: 0; // Icon's own margin would add to the row's 60px gap + position: relative; + overflow: visible; // Icon clips by default; the focus outline extends past the glyph's box color: transparent; + // The star's own glyph must never paint: `-webkit-text-fill-color` wins over any `color` + // a theme sets on focused elements (that would show through the translucent empty color) + -webkit-text-fill-color: transparent; + + // Fill layer: partial stars fill from the reading direction's start (flipped for RTL below) + &::after { + content: attr(data-glyph); + position: absolute; + top: 0; + right: 0; + bottom: 0; + left: 0; + background: linear-gradient( + var(--star-direction, to right), + @star-active-color var(--star-fill, 0%), + var(--star-inactive-color) var(--star-fill, 0%) + ); + -webkit-background-clip: text; + background-clip: text; + color: transparent; + } + + // Focused star only (remote, pointer, tap or drag) + &:focus { + outline: none; // no browser focus ring around the box + } + + // While focused, the fill reaches 0.5px past the glyph's edge (a transparent stroke is still + // painted by the clipped background), covering the outline's anti-aliased inner edge. + // Otherwise the background shows through as a thin dark seam around a filled star. + &:focus::after { + -webkit-text-stroke: 1px transparent; + } + + // Outline layer: a larger box, padded so its glyph sits exactly where the star's glyph is + &:focus::before { + content: attr(data-glyph); + position: absolute; + top: -@star-focus-layer-space; + right: -@star-focus-layer-space; + bottom: -@star-focus-layer-space-bottom; + left: -@star-focus-layer-space; + padding: @star-focus-layer-space @star-focus-layer-space @star-focus-layer-space-bottom; + color: @star-knockout-key; + -webkit-text-fill-color: @star-knockout-key; + .star-outline-shadows(@star-focus-outline-width; @star-focus-outline-color); + // Shadow first (from the whole shape), then remove the glyph's own shape. The shadow + // under the glyph is removed with it, so nothing changes inside the star. + filter: @star-focus-shadow @star-knockout-filter; + } + + // Disabled: 30% opacity (Limestone's @lime-disabled-opacity), also while focused + .disabled({ + .lime-disabled-colors(); + }); }); } @@ -29,10 +144,4 @@ // so partial stars must also fill from the right :global(.enact-locale-right-to-left) .star { --star-direction: to left; -} - -// When any star in the row has focus (remote, tap or drag), -// every highlighted star glows -:focus-within > .star.highlighted { - filter: drop-shadow(0 0 18px rgba(255, 255, 255, 0.8)); } \ No newline at end of file diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index 8bdc72cc3..b38518af0 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -61,7 +61,7 @@ const getStarFromTouch = (ev, id) => { const getStarNumber = (starId, id) => Number(starId.slice(`${id}-star-`.length)); /** - * Moves focus to a star, which shows the focus glow on the row. + * Moves focus to a star, which shows its focus outline. * * Uses Spotlight so remote navigation continues from this star, and falls back to native focus * if Spotlight declines. @@ -123,6 +123,11 @@ const toTenths = (value, max, readOnly) => { * `max`, starting at 3 stars when no value is given. The user selects a star with the * remote, a click, a tap or a touch drag, and `onChange` is called with the star's number. * + * `disabled` works in both modes: the rating is still shown, at 30% opacity, but cannot be + * changed. `overlayMode` switches the color of the empty parts for light overlay surfaces. + * + * Laid out as a 990 × 240 row (UHD px) with a 60px gap between 108px stars, sized for 5 stars. + * * @class StarRatingsBase * @memberof limestone/StarRatings * @ui @@ -144,9 +149,25 @@ const StarRatingsBase = kind({ */ id: PropTypes.string.isRequired, + /** + * Disables StarRatings and makes it non-interactive. + * + * The rating is still shown, at 30% opacity. No star can be selected and `onChange` is not + * called. Like other disabled Limestone controls, the stars can still receive focus with + * the remote while selecting, so the user can see the rating. + * + * @type {Boolean} + * @default false + * @public + */ + disabled: PropTypes.bool, + /** * The number of stars. * + * The row has a fixed 990px width (UHD px), designed for 5 stars; with more stars they + * overflow it. + * * @type {Number} * @default 5 * @public @@ -158,19 +179,31 @@ const StarRatingsBase = kind({ * * Receives an object whose `value` is the number of the selected star, from `1` to `max`. * During a touch drag, it is called whenever the finger moves onto a star other than the - * current `value`. Never called while `readOnly` is `true`. + * current `value`. Never called while `disabled` or `readOnly` is `true`. * * @type {Function} * @public */ onChange: PropTypes.func, + /** + * Colors the empty (unselected) part of the stars for the surface behind them. + * + * * `false`: `#FFFFFF33` (white at 20%), for dark backgrounds. + * * `true`: `#1111111A` (near-black at 10%), for light overlay surfaces. + * + * @type {Boolean} + * @default false + * @public + */ + overlayMode: PropTypes.bool, + /** * Displays the rating without letting the user change it. * * * `true`: the stars cannot be focused or selected, and `value` keeps one decimal place. - * * `false`: the user can select a star, `value` is shown as whole stars, and the filled - * stars glow while the row has focus. + * * `false`: the user can select a star, `value` is shown as whole stars, and the focused + * star shows a white outline and a drop shadow that follow its shape. * * @type {Boolean} * @default false @@ -181,8 +214,8 @@ const StarRatingsBase = kind({ /** * The rating. * - * * Read-only: rounded half-up to one decimal place and clamped to `0`–`max`. When not - * set or `NaN`, no star is filled. + * * Read-only: rounded half-up to one decimal place and clamped to `0`–`max`. Each tenth + * of a star fills an equal share of its area. When not set or `NaN`, no star is filled. * * Selecting: rounded half-up to a whole star and clamped to `1`–`max`, so `0` shows 1 * star. When neither `value` nor `defaultValue` is set (or the value is `NaN`), shows * 3 stars, the spec's recommended default. @@ -197,7 +230,9 @@ const StarRatingsBase = kind({ }, defaultProps: { + disabled: false, max: 5, + overlayMode: false, readOnly: false }, @@ -209,6 +244,7 @@ const StarRatingsBase = kind({ handlers: { // Star forwards {id, ev}; turn it into {value} for onChange onStarClick: handle( + forProp('disabled', false), forProp('readOnly', false), adaptEvent( ({id: starId}, {id}) => ({value: getStarNumber(starId, id)}), @@ -216,9 +252,10 @@ const StarRatingsBase = kind({ ) ), - // Glow starts as soon as the finger touches a star + // The touched star gets focus (and its focus outline) as soon as the finger lands onTouchStart: handle( forward('onTouchStart'), + forProp('disabled', false), forProp('readOnly', false), (ev, {id}) => { focusStar(getStarFromTouch(ev, id)); @@ -229,6 +266,7 @@ const StarRatingsBase = kind({ // Drag across the stars to change the rating onTouchMove: handle( forward('onTouchMove'), + forProp('disabled', false), forProp('readOnly', false), (ev, {id, onChange, value}) => { const star = getStarFromTouch(ev, id); @@ -246,17 +284,29 @@ const StarRatingsBase = kind({ }, computed: { - className: ({readOnly, styler}) => styler.append({readOnly}), + className: ({disabled, readOnly, styler}) => styler.append({disabled, readOnly}), tenths: ({max, readOnly, value}) => toTenths(value, max, readOnly) }, - render: ({id, max, onStarClick, onTouchMove, onTouchStart, readOnly, tenths, ...rest}) => { + render: ({ + disabled, + id, + max, + onStarClick, + onTouchMove, + onTouchStart, + overlayMode, + readOnly, + tenths, + ...rest + }) => { delete rest.onChange; delete rest.value; return (
0} + disabled={disabled} + overlayMode={overlayMode} readOnly={readOnly} value={starValue} onClick={onStarClick} @@ -339,4 +390,4 @@ export { StarRatings, StarRatingsBase, StarRatingsDecorator -}; +}; \ No newline at end of file diff --git a/StarRatings/StarRatings.module.less b/StarRatings/StarRatings.module.less index 14236caca..3c7e1c440 100644 --- a/StarRatings/StarRatings.module.less +++ b/StarRatings/StarRatings.module.less @@ -1,16 +1,17 @@ +// Sizes are design px (UHD); Enact converts them to rem. .starRatings { display: inline-flex; align-items: center; + justify-content: center; // 5 stars + 4 gaps = 780px, centered in the 990px row + width: 990px; + height: 240px; + gap: 60px; -webkit-user-select: none; user-select: none; - // While selecting, horizontal finger movement drives the rating - // instead of scrolling; vertical page scrolling still works - &:not(.readOnly) { + // While selecting (and not disabled), horizontal finger movement drives the + // rating instead of scrolling; vertical page scrolling still works + &:not(.readOnly):not(.disabled) { touch-action: pan-y; } - - > * { - margin: 0 12px; - } } \ No newline at end of file diff --git a/StarRatings/tests/Star-specs.js b/StarRatings/tests/Star-specs.js index 7592eb90d..21e50df20 100644 --- a/StarRatings/tests/Star-specs.js +++ b/StarRatings/tests/Star-specs.js @@ -2,6 +2,7 @@ import '@testing-library/jest-dom'; import {fireEvent, render} from '@testing-library/react'; import Icon from '../../Icon'; +import iconList from '../../Icon/IconList'; import Star from '../Star'; const ID = 'star'; @@ -23,6 +24,13 @@ describe('Star', () => { expect(getStar().textContent).toBe(expectedGlyph); }); + // The fill layer (`::after`) draws this glyph on top of the focus outline + test('should pass the star glyph to the fill layer as `data-glyph`', () => { + render(); + + expect(getStar()).toHaveAttribute('data-glyph', String.fromCodePoint(iconList.star)); + }); + test('should apply `id` to the root element', () => { render(); @@ -42,11 +50,10 @@ describe('Star', () => { expect(getStar()).toHaveClass('custom'); }); - test('should not pass `value` or `highlighted` to the DOM', () => { - render(); + test('should not pass `value` to the DOM', () => { + render(); expect(getStar()).not.toHaveAttribute('value'); - expect(getStar()).not.toHaveAttribute('highlighted'); }); }); @@ -57,11 +64,18 @@ describe('Star', () => { expect(getFill()).toBe('0%'); }); + // 0.1 and 0.9 are pinned to 20% and 80%; the steps between give equal shares of the area test.each([ [0, '0%'], - [0.1, '10%'], + [0.1, '20%'], + [0.2, '33%'], + [0.3, '39%'], + [0.4, '45%'], [0.5, '50%'], - [0.7, '70%'], + [0.6, '55%'], + [0.7, '61%'], + [0.8, '67%'], + [0.9, '80%'], [1, '100%'] ])('should fill %p as %s', (value, expected) => { render(); @@ -71,10 +85,10 @@ describe('Star', () => { test.each([ [0.04, '0%'], - [0.05, '10%'], + [0.05, '20%'], [0.54, '50%'], - [0.55, '60%'], - [0.94, '90%'], + [0.55, '55%'], + [0.94, '80%'], [0.95, '100%'] ])('should round %p half-up to %s', (value, expected) => { render(); @@ -107,32 +121,78 @@ describe('Star', () => { }); }); - describe('`highlighted`', () => { - test('should not have the `highlighted` class by default', () => { + describe('`readOnly`', () => { + test('should be focusable by default', () => { render(); - expect(getStar()).not.toHaveClass('highlighted'); + expect(getStar()).toHaveClass('spottable'); }); - test('should add the `highlighted` class when true', () => { - render(); + test('should not be focusable when true', () => { + render(); - expect(getStar()).toHaveClass('highlighted'); + expect(getStar()).not.toHaveClass('spottable'); }); }); - describe('`readOnly`', () => { - test('should be focusable by default', () => { + describe('`overlayMode`', () => { + test('should not have the `overlayMode` class by default', () => { + render(); + + expect(getStar()).not.toHaveClass('overlayMode'); + }); + + test('should add the `overlayMode` class when true', () => { + render(); + + expect(getStar()).toHaveClass('overlayMode'); + }); + + test('should not pass `overlayMode` to the DOM', () => { + render(); + + expect(getStar()).not.toHaveAttribute('overlayMode'); + expect(getStar()).not.toHaveAttribute('overlaymode'); + }); + }); + + describe('`disabled`', () => { + test('should not be disabled by default', () => { render(); + expect(getStar()).not.toHaveAttribute('disabled'); + expect(getStar()).toHaveAttribute('aria-disabled', 'false'); + }); + + test('should set `disabled` and `aria-disabled` when true', () => { + render(); + + expect(getStar()).toHaveAttribute('disabled'); + expect(getStar()).toHaveAttribute('aria-disabled', 'true'); + }); + + // Limestone convention: disabled controls can still receive focus + test('should stay focusable when true', () => { + render(); + expect(getStar()).toHaveClass('spottable'); }); - test('should not be focusable when true', () => { - render(); + test('should be disabled and not focusable when also read-only', () => { + render(); + expect(getStar()).toHaveAttribute('disabled'); expect(getStar()).not.toHaveClass('spottable'); }); + + test('should not call `onClick` when true', () => { + const handleClick = jest.fn(); + render(); + + fireEvent.click(getStar()); + + expect(handleClick).not.toHaveBeenCalled(); + }); }); describe('`onClick`', () => { diff --git a/StarRatings/tests/StarRatings-specs.js b/StarRatings/tests/StarRatings-specs.js index 1a3ff302e..dd0d0bb14 100644 --- a/StarRatings/tests/StarRatings-specs.js +++ b/StarRatings/tests/StarRatings-specs.js @@ -1,7 +1,8 @@ import '@testing-library/jest-dom'; import {fireEvent, render} from '@testing-library/react'; -import StarRatings from '../StarRatings'; +import {FILL_STEPS} from '../Star'; +import StarRatings, {StarRatingsBase} from '../StarRatings'; const ID = 'rating'; @@ -12,8 +13,13 @@ const getStars = () => Array.from(document.querySelectorAll(`[id^="${ID}-star-"] // Fill of each star as rendered, e.g. ['100%', '100%', '50%', '0%', '0%'] const getFills = () => getStars().map((star) => star.style.getPropertyValue('--star-fill')); -// Total number of stars filled, e.g. 2.5 (sums whole percents to avoid float error) -const getFilledTotal = () => getFills().reduce((sum, fill) => sum + parseInt(fill), 0) / 100; +// Total number of stars filled, e.g. 2.5: each star's fill position is mapped back to its tenth +// (sums whole tenths to avoid float error) +const getFilledTotal = () => getFills().reduce((sum, fill) => { + const tenths = FILL_STEPS.indexOf(parseInt(fill)); + if (tenths < 0) throw new Error(`unexpected --star-fill ${fill}`); + return sum + tenths; +}, 0) / 10; describe('StarRatings', () => { describe('read-only rounding (spec: half-up to one decimal place)', () => { @@ -28,7 +34,7 @@ describe('StarRatings', () => { test('should fill 2.6 stars for 2.55', () => { render(); - expect(getFills()).toEqual(['100%', '100%', '60%', '0%', '0%']); + expect(getFills()).toEqual(['100%', '100%', '55%', '0%', '0%']); expect(getFilledTotal()).toBe(2.6); }); @@ -146,6 +152,19 @@ describe('StarRatings', () => { expect(getStars()).toHaveLength(5); }); + test('should not use `overlayMode` on the stars by default', () => { + render(); + + getStars().forEach((star) => expect(star).not.toHaveClass('overlayMode')); + }); + + test('should pass `overlayMode` to every star', () => { + render(); + + getStars().forEach((star) => expect(star).toHaveClass('overlayMode')); + expect(document.getElementById(ID)).not.toHaveAttribute('overlaymode'); + }); + test('should render `max` stars', () => { render(); @@ -173,12 +192,6 @@ describe('StarRatings', () => { getStars().forEach((star) => expect(star).not.toHaveClass('spottable')); }); - test('should not highlight any star', () => { - render(); - - getStars().forEach((star) => expect(star).not.toHaveClass('highlighted')); - }); - test('should not call `onChange` or change the value on click', () => { const handleChange = jest.fn(); render(); @@ -197,16 +210,6 @@ describe('StarRatings', () => { getStars().forEach((star) => expect(star).toHaveClass('spottable')); }); - test('should highlight only filled stars', () => { - render(); - - expect(getStar(1)).toHaveClass('highlighted'); - expect(getStar(2)).toHaveClass('highlighted'); - expect(getStar(3)).toHaveClass('highlighted'); - expect(getStar(4)).not.toHaveClass('highlighted'); - expect(getStar(5)).not.toHaveClass('highlighted'); - }); - test('should call `onChange` with the number of the clicked star', () => { const handleChange = jest.fn(); render(); @@ -236,6 +239,59 @@ describe('StarRatings', () => { }); }); + describe('disabled', () => { + test('should not be disabled by default', () => { + render(); + + expect(document.getElementById(ID)).not.toHaveClass('disabled'); + getStars().forEach((star) => expect(star).not.toHaveAttribute('disabled')); + }); + + test('should mark the component and every star as disabled', () => { + render(); + + const row = document.getElementById(ID); + expect(row).toHaveClass('disabled'); + expect(row).toHaveAttribute('aria-disabled', 'true'); + getStars().forEach((star) => expect(star).toHaveAttribute('disabled')); + }); + + test('should still show the rating', () => { + render(); + + expect(getFilledTotal()).toBe(4); + }); + + test('should still show decimals when also read-only', () => { + render(); + + expect(getFilledTotal()).toBe(2.6); + }); + + // Limestone convention: disabled controls can still receive focus + test('should keep the stars focusable while selecting', () => { + render(); + + getStars().forEach((star) => expect(star).toHaveClass('spottable')); + }); + + test('should not make the stars focusable when also read-only', () => { + render(); + + getStars().forEach((star) => expect(star).not.toHaveClass('spottable')); + }); + + test('should not call `onChange` or change the value on click', () => { + const handleChange = jest.fn(); + render(); + + fireEvent.click(getStar(4)); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getFilledTotal()).toBe(2); + }); + }); + describe('touch drag', () => { const originalElementFromPoint = document.elementFromPoint; @@ -282,6 +338,42 @@ describe('StarRatings', () => { expect(getFilledTotal()).toBe(3); }); + test('should not call `onChange` when disabled', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(4)); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getFilledTotal()).toBe(2); + }); + + // Changeable also blocks `onChange` when disabled, so test the base component on its own + test('should not call `onChange` from `StarRatingsBase` when disabled', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(4)); + + expect(handleChange).not.toHaveBeenCalled(); + }); + + test('should not move focus to the touched star when disabled', () => { + render(); + + touchMoveOver(getStar(4)); + + expect(getStar(4)).not.toHaveFocus(); + }); + + test('should move focus to the touched star while selecting', () => { + render(); + + touchMoveOver(getStar(4)); + + expect(getStar(4)).toHaveFocus(); + }); + test('should not call `onChange` when read-only', () => { const handleChange = jest.fn(); render(); diff --git a/samples/sampler/stories/default/Star.js b/samples/sampler/stories/default/Star.js index 52b10ce41..6ce6bf6cb 100644 --- a/samples/sampler/stories/default/Star.js +++ b/samples/sampler/stories/default/Star.js @@ -16,8 +16,10 @@ export const _Star = (args) => { }; const controls = { - active: args['active'], + disabled: args['disabled'], highlighted: args['highlighted'], + overlayMode: args['overlayMode'], + readOnly: args['readOnly'], value: args['value'] }; @@ -30,8 +32,10 @@ export const _Star = (args) => { ); }; -boolean('active', _Star, StarConfig); +boolean('disabled', _Star, StarConfig); boolean('highlighted', _Star, StarConfig); +boolean('overlayMode', _Star, StarConfig); +boolean('readOnly', _Star, StarConfig); range('value', _Star, StarConfig, {min: 0, max: 1, step: 0.01}); _Star.storyName = 'Star'; diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index ec2d6184a..6067180db 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -15,13 +15,15 @@ export const _StarRatings = (args) => { const [defaultValue, setDefaultValue] = useState(0); // eslint-disable-line react-hooks/rules-of-hooks const handleValue = useCallback((event) => { // eslint-disable-line react-hooks/rules-of-hooks - action('onChange'); + action('onChange')(event); setDefaultValue(event.value); }, []); const controls = { - value: args['readOnly'] ? args['value'] : defaultValue, - readOnly: args['readOnly'] + disabled: args['disabled'], + overlayMode: args['overlayMode'], + readOnly: args['readOnly'], + value: args['readOnly'] ? args['value'] : defaultValue }; return (<> @@ -33,7 +35,9 @@ export const _StarRatings = (args) => { ); }; +boolean('disabled', _StarRatings, StarRatingsConfig); boolean('readOnly', _StarRatings, StarRatingsConfig); +boolean('overlayMode', _StarRatings, StarRatingsConfig); range('value', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}); _StarRatings.storyName = 'StarRatings'; From e81344220b476eb294d9ca679072f1ea52df70c0 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:06:55 +0300 Subject: [PATCH 09/17] Added changelog entry --- CHANGELOG.md | 5 +++++ StarRatings/Star.js | 2 +- StarRatings/StarRatings.js | 24 ++++++++++++------------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e5441578..9d5c2662e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ The following is a curated list of changes in the Enact limestone module, newest ## [unreleased] +### Added + +- new component `StarRatings` + + ### Fixed - `limestone/Alert` Button width for overlay type diff --git a/StarRatings/Star.js b/StarRatings/Star.js index 9f8e42726..32e0ae28f 100644 --- a/StarRatings/Star.js +++ b/StarRatings/Star.js @@ -207,4 +207,4 @@ export { Star, StarBase, StarDecorator -}; \ No newline at end of file +}; diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index b38518af0..9321f6985 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -289,17 +289,17 @@ const StarRatingsBase = kind({ }, render: ({ - disabled, - id, - max, - onStarClick, - onTouchMove, - onTouchStart, - overlayMode, - readOnly, - tenths, - ...rest - }) => { + disabled, + id, + max, + onStarClick, + onTouchMove, + onTouchStart, + overlayMode, + readOnly, + tenths, + ...rest + }) => { delete rest.onChange; delete rest.value; @@ -390,4 +390,4 @@ export { StarRatings, StarRatingsBase, StarRatingsDecorator -}; \ No newline at end of file +}; From e00fa7e2dddc858c4e36e482bae27ed0140de30f Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:04:00 +0300 Subject: [PATCH 10/17] Added screenshot and ui tests --- tests/screenshot/apps/LimestoneComponents.js | 2 + .../screenshot/apps/components/StarRatings.js | 57 +++++++ tests/screenshot/apps/importer.js | 2 + tests/ui/apps/StarRatings/StarRatings-View.js | 47 +++++ .../ui/specs/StarRatings/StarRatings-specs.js | 160 ++++++++++++++++++ tests/ui/specs/StarRatings/StarRatingsPage.js | 81 +++++++++ 6 files changed, 349 insertions(+) create mode 100644 tests/screenshot/apps/components/StarRatings.js create mode 100644 tests/ui/apps/StarRatings/StarRatings-View.js create mode 100644 tests/ui/specs/StarRatings/StarRatings-specs.js create mode 100644 tests/ui/specs/StarRatings/StarRatingsPage.js diff --git a/tests/screenshot/apps/LimestoneComponents.js b/tests/screenshot/apps/LimestoneComponents.js index dd748e224..3ae072e64 100644 --- a/tests/screenshot/apps/LimestoneComponents.js +++ b/tests/screenshot/apps/LimestoneComponents.js @@ -47,6 +47,7 @@ import Scroller from './components/Scroller'; import Slider from './components/Slider'; import Spinner from './components/Spinner'; import Sprite from './components/Sprite'; +import StarRatings from './components/StarRatings'; import Steps from './components/Steps'; import Switch from './components/Switch'; import SwitchItem from './components/SwitchItem'; @@ -106,6 +107,7 @@ const components = { Slider, Spinner, Sprite, + StarRatings, Steps, Switch, SwitchItem, diff --git a/tests/screenshot/apps/components/StarRatings.js b/tests/screenshot/apps/components/StarRatings.js new file mode 100644 index 000000000..3e90cc3a9 --- /dev/null +++ b/tests/screenshot/apps/components/StarRatings.js @@ -0,0 +1,57 @@ +import StarRatings from '../../../../StarRatings'; + +import {withConfig} from './utils'; + +const starRatingsSmokeTests = [ + // Selecting: no value shows the default 3 stars + , + , + , + // Read-only: whole, half and decimal ratings + , + , + , + , + // Disabled + , + +]; + +// Every tenth of a star, to check the fill steps +const starRatingsFillStepTests = [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9].map((value) => ( + +)); + +// Focus lands on the first star. Values differ from the smoke tests so each screenshot is unique. +const starRatingsFocusTests = [ + , + , + +]; + +const starRatingsOverlayModeTests = [ + , + , + +]; + +const starRatingsRtlTests = [ + , + +]; + +const StarRatingsTests = [ + ...starRatingsSmokeTests, + ...starRatingsFillStepTests, + ...withConfig({focus: true}, starRatingsFocusTests), + ...withConfig({wrapper: {light: true}}, starRatingsOverlayModeTests), + ...withConfig({wrapper: {light: true}, focus: true}, [ + + ]), + ...withConfig({locale: 'ar-SA'}, starRatingsRtlTests), + ...withConfig({locale: 'ar-SA', focus: true}, [ + + ]) +]; + +export default StarRatingsTests; diff --git a/tests/screenshot/apps/importer.js b/tests/screenshot/apps/importer.js index 947dc5df7..1d16fbeb9 100644 --- a/tests/screenshot/apps/importer.js +++ b/tests/screenshot/apps/importer.js @@ -39,6 +39,7 @@ import Scroller from '../../../Scroller'; import Slider from '../../../Slider'; import Spinner from '../../../Spinner'; import Sprite from '../../../Sprite'; +import StarRatings from '../../../StarRatings'; import Steps from '../../../Steps'; import Switch from '../../../Switch'; import SwitchItem from '../../../SwitchItem'; @@ -93,6 +94,7 @@ const components = { Slider, Spinner, Sprite, + StarRatings, Steps, Switch, SwitchItem, diff --git a/tests/ui/apps/StarRatings/StarRatings-View.js b/tests/ui/apps/StarRatings/StarRatings-View.js new file mode 100644 index 000000000..97ee3fb95 --- /dev/null +++ b/tests/ui/apps/StarRatings/StarRatings-View.js @@ -0,0 +1,47 @@ +import spotlight from '@enact/spotlight'; +import {useCallback, useState} from 'react'; + +import StarRatings from '../../../../StarRatings'; +import ThemeDecorator from '../../../../ThemeDecorator'; + +// NOTE: Forcing pointer mode off so we can be sure that regardless of webOS pointer mode the app +// runs the same way +spotlight.setPointerMode(false); + +const App = (props) => { + const [lastChange, setLastChange] = useState('none'); + + const handleChange = useCallback(({value}) => { + setLastChange(String(value)); + }, []); + + return ( +
+
+ +
+
+ +
+
+ +
+
{lastChange}
+
+ ); +}; + +export default ThemeDecorator(App); diff --git a/tests/ui/specs/StarRatings/StarRatings-specs.js b/tests/ui/specs/StarRatings/StarRatings-specs.js new file mode 100644 index 000000000..e707f1b62 --- /dev/null +++ b/tests/ui/specs/StarRatings/StarRatings-specs.js @@ -0,0 +1,160 @@ +const Page = require('./StarRatingsPage'); + +describe('StarRatings', function () { + + const { + starRatingsDefault, + starRatingsReadOnly, + starRatingsDisabled + } = Page.components; + + describe('LTR locale', function () { + beforeEach(async function () { + await Page.open(); + }); + + describe('default', function () { + it('should focus the first star on load', async function () { + expect(await starRatingsDefault.star(1).isFocused()).toBe(true); + }); + + it('should show 3 stars when no value is set', async function () { + expect(await starRatingsDefault.filledStars()).toBe(3); + }); + + it('should show a half star in read-only mode', async function () { + expect(await starRatingsReadOnly.fills()).toEqual(['100%', '100%', '100%', '50%', '0%']); + }); + + it('should mark read-only and disabled ratings', async function () { + expect(await starRatingsReadOnly.isReadOnly).toBe(true); + expect(await starRatingsDisabled.isDisabled).toBe(true); + expect(await starRatingsDefault.isReadOnly).toBe(false); + expect(await starRatingsDefault.isDisabled).toBe(false); + }); + + it('should show the focus outline only on the focused star', async function () { + expect(await starRatingsDefault.hasFocusOutline(1)).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(2)).toBe(false); + }); + }); + + describe('5-way', function () { + it('should move focus to the next star', async function () { + await Page.spotlightRight(); + + expect(await starRatingsDefault.star(2).isFocused()).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(2)).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(1)).toBe(false); + }); + + it('should select the focused star', async function () { + await Page.spotlightRight(); + await Page.spotlightRight(); + await Page.spotlightRight(); + await Page.spotlightSelect(); + + expect(await starRatingsDefault.filledStars()).toBe(4); + expect(await Page.lastChange).toBe('4'); + }); + + it('should select a lower star', async function () { + await Page.spotlightSelect(); + + expect(await starRatingsDefault.filledStars()).toBe(1); + expect(await Page.lastChange).toBe('1'); + }); + + it('should not change the fill when focusing an empty star', async function () { + await Page.spotlightRight(); + await Page.spotlightRight(); + await Page.spotlightRight(); + + expect(await starRatingsDefault.star(4).isFocused()).toBe(true); + expect(await starRatingsDefault.fill(4)).toBe('0%'); + }); + + it('should skip the read-only rating and focus the disabled one', async function () { + await Page.spotlightDown(); + + expect(await Page.focusedId).toMatch(/^starRatingsDisabled-star-/); + }); + + it('should not change a disabled rating on select', async function () { + await Page.spotlightDown(); + await Page.spotlightSelect(); + + expect(await starRatingsDisabled.filledStars()).toBe(2); + expect(await Page.lastChange).toBe('none'); + }); + + it('should return to the last focused star', async function () { + await Page.spotlightRight(); + await Page.spotlightRight(); + await Page.spotlightDown(); + + // A rating entered for the first time starts at its first star + expect(await starRatingsDisabled.star(1).isFocused()).toBe(true); + + // Move to the far end, so only "last focused" (not position) leads back to star 3 + await Page.spotlightRight(); + await Page.spotlightRight(); + await Page.spotlightRight(); + await Page.spotlightRight(); + + expect(await starRatingsDisabled.star(5).isFocused()).toBe(true); + + await Page.spotlightUp(); + + expect(await starRatingsDefault.star(3).isFocused()).toBe(true); + }); + }); + + describe('pointer', function () { + it('should select the clicked star', async function () { + await starRatingsDefault.star(5).click(); + + expect(await starRatingsDefault.filledStars()).toBe(5); + expect(await Page.lastChange).toBe('5'); + }); + + it('should not change a read-only rating on click', async function () { + await starRatingsReadOnly.star(1).click(); + + expect(await starRatingsReadOnly.fills()).toEqual(['100%', '100%', '100%', '50%', '0%']); + expect(await Page.lastChange).toBe('none'); + }); + + it('should not change a disabled rating on click', async function () { + await starRatingsDisabled.star(4).click(); + + expect(await starRatingsDisabled.filledStars()).toBe(2); + expect(await Page.lastChange).toBe('none'); + }); + }); + }); + + describe('RTL locale', function () { + beforeEach(async function () { + await Page.open('?locale=ar-SA'); + }); + + it('should place the first star on the right', async function () { + expect(await starRatingsDefault.starLeft(1)).toBeGreaterThan(await starRatingsDefault.starLeft(2)); + }); + + it('should move focus to the next star with the left key', async function () { + expect(await starRatingsDefault.star(1).isFocused()).toBe(true); + + await Page.spotlightLeft(); + + expect(await starRatingsDefault.star(2).isFocused()).toBe(true); + }); + + it('should fill a partial star from the right', async function () { + const direction = await browser.execute((el) => window.getComputedStyle(el).getPropertyValue('--star-direction').trim(), await starRatingsReadOnly.star(4)); + + expect(direction).toBe('to left'); + }); + }); +}); diff --git a/tests/ui/specs/StarRatings/StarRatingsPage.js b/tests/ui/specs/StarRatings/StarRatingsPage.js new file mode 100644 index 000000000..92dd38de0 --- /dev/null +++ b/tests/ui/specs/StarRatings/StarRatingsPage.js @@ -0,0 +1,81 @@ +'use strict'; +const {Page} = require('@enact/ui-test-utils/utils'); + +class StarRatingsInterface { + constructor (id) { + this.id = `${id}`; + } + + get self () { + return $(`#${this.id}`); + } + + get isReadOnly () { + return $(`#${this.id}.StarRatings_StarRatings_readOnly`).isExisting(); + } + + get isDisabled () { + return $(`#${this.id}.StarRatings_StarRatings_disabled`).isExisting(); + } + + star (number) { + return $(`#${this.id}-star-${number}`); + } + + // `--star-fill` of a star, e.g. '100%', '50%' or '0%' + async fill (number) { + return await browser.execute((el) => el.style.getPropertyValue('--star-fill'), await this.star(number)); + } + + async fills () { + const fills = []; + for (let number = 1; number <= 5; number++) { + fills.push(await this.fill(number)); + } + return fills; + } + + async filledStars () { + return (await this.fills()).filter((fill) => fill === '100%').length; + } + + // The focus outline is drawn by the focused star's `::before` + async hasFocusOutline (number) { + return await browser.execute((el) => window.getComputedStyle(el, '::before').content !== 'none', await this.star(number)); + } + + async starLeft (number) { + return (await this.star(number).getLocation()).x; + } +} + +class StarRatingsPage extends Page { + constructor () { + super(); + this.title = 'StarRatings Test'; + const starRatingsDefault = new StarRatingsInterface('starRatingsDefault'); + const starRatingsReadOnly = new StarRatingsInterface('starRatingsReadOnly'); + const starRatingsDisabled = new StarRatingsInterface('starRatingsDisabled'); + + this.components = { + starRatingsDefault, + starRatingsReadOnly, + starRatingsDisabled + }; + } + + // Value from the last `onChange` call, or 'none' + get lastChange () { + return $('#lastChange').getText(); + } + + get focusedId () { + return browser.execute(() => document.activeElement && document.activeElement.id); + } + + async open (urlExtra) { + await super.open('StarRatings-View', urlExtra); + } +} + +module.exports = new StarRatingsPage(); From 1ff6cd2dba46586609e104d889065045848e4fb3 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:27:00 +0300 Subject: [PATCH 11/17] Review fixes --- StarRatings/StarRatings.js | 228 ++++++++++-- StarRatings/StarRatings.module.less | 7 +- StarRatings/tests/StarRatings-specs.js | 349 +++++++++++++++++- .../sampler/stories/default/StarRatings.js | 4 +- .../screenshot/apps/components/StarRatings.js | 10 +- tests/ui/apps/StarRatings/StarRatings-View.js | 19 +- .../ui/specs/StarRatings/StarRatings-specs.js | 94 +++-- tests/ui/specs/StarRatings/StarRatingsPage.js | 18 +- 8 files changed, 670 insertions(+), 59 deletions(-) diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index 9321f6985..65f67c0fc 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -25,6 +25,7 @@ import Spotlight from '@enact/spotlight'; import SpotlightContainerDecorator from '@enact/spotlight/SpotlightContainerDecorator'; import Changeable from '@enact/ui/Changeable'; import PropTypes from 'prop-types'; +import {useCallback, useEffect, useRef, useState} from 'react'; import clamp from 'ramda/src/clamp'; import compose from 'ramda/src/compose'; @@ -113,6 +114,137 @@ const toTenths = (value, max, readOnly) => { return clamp(1, max, Math.round(rating)) * 10; }; +/** + * Selects the star under the finger: focuses it and, if it differs from the rating shown, calls + * `onChange` with its number. Shared by `touchstart` (spec: pressing a star selects it right + * away) and `touchmove` (dragging updates the rating). + * + * Compares with the shown rating rather than `value`, so a missing value (3 stars shown) or a + * clamped one does not fire `onChange` for the star that is already shown. + * + * @param {TouchEvent} ev The touch event. + * @param {Object} props The `StarRatingsBase` props. + * @returns {Boolean} `false` when the finger is not over a star, so the last value is kept. + * @private + */ +const selectTouchedStar = (ev, {id, max, onChange, readOnly, value}) => { + const star = getStarFromTouch(ev, id); + if (!star) return false; // outside the row: keep the last value and focus + + focusStar(star); + + const shown = toTenths(value, max, readOnly) / 10; + const starNumber = getStarNumber(star.id, id); + if (onChange && starNumber && starNumber !== shown) { + onChange({value: starNumber}); + } + return true; +}; + +/** + * Selector of the selected star, which Spotlight focuses when the row is entered for the first + * time, so pressing Enter right away keeps the rating that is shown. + * + * @type {String} + * @private + */ +const SELECTED_STAR_SELECTOR = '[data-star-selected]'; + +/** + * Tells the app about the rating the stars show, when it differs from `value`. + * + * While selecting, the stars show a whole rating from 1 to `max`, so a missing value (3 stars), + * `0` (1 star), a value above `max` (`max` stars) or a fraction (rounded) would show a rating + * the app does not have. In that case `onChange` is called once with `{value: shown, normalized: + * true}`, so the app (and `Changeable`) holds what the user sees, and pressing Done in a popup + * submits it. `normalized` lets an app that acts on each selection (e.g. a popup that closes on + * selection) ignore this call. + * + * Skipped while read-only (no rating is chosen) or disabled (`Changeable` ignores changes then); + * it runs again when either becomes `false`. + * + * @param {Object} props The `StarRatings` props. + * @returns {undefined} + * @private + */ +const useSyncShownValue = ({disabled = false, max = 5, onChange, readOnly = false, value}) => { + const synced = useRef(null); + const shown = toTenths(value, max, readOnly) / 10; + + useEffect(() => { + if (readOnly || disabled || !onChange || value === shown) { + synced.current = null; + return; + } + + // Calls once per mismatch, even if the app ignores it or the effect runs twice (Strict Mode) + const key = `${value}:${shown}`; + if (synced.current === key) return; + synced.current = key; + + onChange({value: shown, normalized: true}); + }, [disabled, onChange, readOnly, shown, value]); +}; + +/** + * Applies `useSyncShownValue`. Placed inside `Changeable`, so the reported rating is stored. + * + * @hoc + * @param {Component} Wrapped The component to wrap. + * @returns {Component} The wrapped component. + * @private + */ +const ShownValueDecorator = (Wrapped) => { + const ShownValue = (props) => { + useSyncShownValue(props); + + return ; + }; + + return ShownValue; +}; + +/** + * Tracks which star has focus and passes its number to `StarRatingsBase` as `focusedStar`, so + * the stars up to and including the focused one are filled as a preview. + * + * Only the fill changes: `value` and `onChange` are untouched until the user selects the star. + * When focus leaves the row, `focusedStar` is cleared and the selected rating shows again. + * + * @hoc + * @param {Component} Wrapped The component to wrap. + * @returns {Component} The wrapped component. + * @private + */ +const FocusPreviewDecorator = (Wrapped) => { + const FocusPreview = ({onBlur, onFocus, ...rest}) => { + const {id} = rest; + const [focusedStar, setFocusedStar] = useState(null); + + const handleFocus = useCallback((ev) => { + if (onFocus) onFocus(ev); + + // Focus events bubble from the stars; anything else in the row is ignored + const starId = ev.target && ev.target.id; + const isStar = typeof starId === 'string' && starId.startsWith(`${id}-star-`); + setFocusedStar(isStar ? getStarNumber(starId, id) || null : null); + }, [id, onFocus]); + + const handleBlur = useCallback((ev) => { + if (onBlur) onBlur(ev); + + // Moving between stars fires blur then focus; only clear when focus leaves the row + if (!ev.relatedTarget || !ev.currentTarget.contains(ev.relatedTarget)) { + setFocusedStar(null); + } + }, [onBlur]); + + return ; + }; + + return FocusPreview; +}; + /** * A star rating component, without any behaviors applied. * @@ -122,11 +254,18 @@ const toTenths = (value, max, readOnly) => { * * Selecting (`readOnly` is `false`, the default): shows `value` as whole stars, from 1 to * `max`, starting at 3 stars when no value is given. The user selects a star with the * remote, a click, a tap or a touch drag, and `onChange` is called with the star's number. + * [StarRatings]{@link limestone/StarRatings.StarRatings} also reports the shown rating + * with `onChange` when it differs from `value`, and enters the row at the selected star, so + * pressing Enter or Done keeps the rating the user sees. While a star has focus, the stars + * up to and including it are filled as a preview (`value` changes only on selection); the + * selected rating shows again when focus leaves the row. * * `disabled` works in both modes: the rating is still shown, at 30% opacity, but cannot be * changed. `overlayMode` switches the color of the empty parts for light overlay surfaces. * - * Laid out as a 990 × 240 row (UHD px) with a 60px gap between 108px stars, sized for 5 stars. + * Laid out as a row of 108px stars with a 60px gap (UHD px). The row is as wide as its stars, + * for any `max`; space around it, such as the design's 990 × 240 area in a popup, is up to the + * surrounding layout. * * @class StarRatingsBase * @memberof limestone/StarRatings @@ -162,11 +301,23 @@ const StarRatingsBase = kind({ */ disabled: PropTypes.bool, + /** + * The number of the focused star, set by + * [StarRatingsDecorator]{@link limestone/StarRatings.StarRatingsDecorator}. + * + * While selecting (and not disabled), the stars up to and including this one are filled + * as a preview, up or down from the selected rating. `value` does not change until the + * user selects the star. + * + * @type {Number} + * @private + */ + focusedStar: PropTypes.number, + /** * The number of stars. * - * The row has a fixed 990px width (UHD px), designed for 5 stars; with more stars they - * overflow it. + * Set it for each scenario. The row grows or shrinks with the number of stars. * * @type {Number} * @default 5 @@ -178,8 +329,18 @@ const StarRatingsBase = kind({ * Called when the user selects a star. * * Receives an object whose `value` is the number of the selected star, from `1` to `max`. - * During a touch drag, it is called whenever the finger moves onto a star other than the - * current `value`. Never called while `disabled` or `readOnly` is `true`. + * With touch, it is called as soon as the finger lands on a star other than the one + * shown, and again whenever a drag moves it onto another star. Lifting the finger, or + * moving it off the stars, keeps the last value. + * + * With [StarRatings]{@link limestone/StarRatings.StarRatings} (not `StarRatingsBase`), + * also called once, with `{value, normalized: true}`, when the rating shown while + * selecting differs from `value`: when no value is set (3), for `0` (1), above `max` + * (`max`) or for a fraction (rounded). This way the app holds the rating the user sees, + * e.g. to submit it with a popup's Done button. An app that acts on every selection, + * such as a popup that closes on selection, can ignore events with `normalized` set. + * + * Never called while `disabled` or `readOnly` is `true`. * * @type {Function} * @public @@ -218,7 +379,8 @@ const StarRatingsBase = kind({ * of a star fills an equal share of its area. When not set or `NaN`, no star is filled. * * Selecting: rounded half-up to a whole star and clamped to `1`–`max`, so `0` shows 1 * star. When neither `value` nor `defaultValue` is set (or the value is `NaN`), shows - * 3 stars, the spec's recommended default. + * 3 stars, the spec's recommended default. If the shown rating differs from `value`, + * `onChange` reports it (see `onChange`). * * If not set, the component keeps its own value and updates it on each selection; use * `defaultValue` to set where it starts. @@ -252,15 +414,12 @@ const StarRatingsBase = kind({ ) ), - // The touched star gets focus (and its focus outline) as soon as the finger lands + // Spec: pressing a star selects it right away (focus outline and fill together) onTouchStart: handle( forward('onTouchStart'), forProp('disabled', false), forProp('readOnly', false), - (ev, {id}) => { - focusStar(getStarFromTouch(ev, id)); - return true; - } + selectTouchedStar ), // Drag across the stars to change the rating @@ -268,16 +427,17 @@ const StarRatingsBase = kind({ forward('onTouchMove'), forProp('disabled', false), forProp('readOnly', false), - (ev, {id, onChange, value}) => { - const star = getStarFromTouch(ev, id); - if (!star) return false; // outside the row: keep the last value and focus - - focusStar(star); + selectTouchedStar + ), - const starNumber = getStarNumber(star.id, id); - if (onChange && starNumber && starNumber !== value) { - onChange({value: starNumber}); - } + // The touch already selected the star, so cancel the click the browser would send on + // release; otherwise `onChange` would be called a second time for the same star + onTouchEnd: handle( + forward('onTouchEnd'), + forProp('disabled', false), + forProp('readOnly', false), + (ev) => { + if (ev.cancelable) ev.preventDefault(); return true; } ) @@ -285,7 +445,13 @@ const StarRatingsBase = kind({ computed: { className: ({disabled, readOnly, styler}) => styler.append({disabled, readOnly}), - tenths: ({max, readOnly, value}) => toTenths(value, max, readOnly) + // The selected rating, which Spotlight enters the row at + selected: ({max, readOnly, value}) => (readOnly ? null : toTenths(value, max, readOnly) / 10), + // What the stars show: the focus preview while selecting, otherwise `value` + tenths: ({disabled, focusedStar, max, readOnly, value}) => { + const preview = !readOnly && !disabled && focusedStar; + return toTenths(preview ? focusedStar : value, max, readOnly); + } }, render: ({ @@ -293,13 +459,16 @@ const StarRatingsBase = kind({ id, max, onStarClick, + onTouchEnd, onTouchMove, onTouchStart, overlayMode, readOnly, + selected, tenths, ...rest }) => { + delete rest.focusedStar; delete rest.onChange; delete rest.value; @@ -308,6 +477,7 @@ const StarRatingsBase = kind({ {...rest} aria-disabled={disabled} id={id} + onTouchEnd={onTouchEnd} onTouchMove={onTouchMove} onTouchStart={onTouchStart} > @@ -317,6 +487,7 @@ const StarRatingsBase = kind({ return ( { test('should call `onChange` with the number of the clicked star', () => { const handleChange = jest.fn(); - render(); + render(); fireEvent.click(getStar(3)); expect(handleChange).toHaveBeenCalledTimes(1); expect(handleChange).toHaveBeenCalledWith(expect.objectContaining({value: 3})); + expect(handleChange.mock.calls[0][0].normalized).toBeUndefined(); }); test('should remember the clicked star when `value` is not set', () => { @@ -239,6 +240,242 @@ describe('StarRatings', () => { }); }); + // Spec: selecting starts at 3 stars; the app must hold the rating the user sees + describe('shown value sync', () => { + test('should report the default 3 stars when no value is set', () => { + const handleChange = jest.fn(); + render(); + + expect(handleChange).toHaveBeenCalledTimes(1); + expect(handleChange).toHaveBeenCalledWith({value: 3, normalized: true}); + expect(getFilledTotal()).toBe(3); + }); + + test.each([ + ['0', {value: 0}, 1], + ['a value above max', {value: 7}, 5], + ['a fraction', {value: 2.6}, 3], + ['NaN', {value: NaN}, 3], + ['a default value of 0', {defaultValue: 0}, 1], + ['the default above a smaller max', {max: 2}, 2] + ])('should report the shown rating for %s', (_, props, expected) => { + const handleChange = jest.fn(); + render(); + + expect(handleChange).toHaveBeenCalledTimes(1); + expect(handleChange).toHaveBeenCalledWith({value: expected, normalized: true}); + }); + + test('should store the reported rating when `value` is not set', () => { + const handleChange = jest.fn(); + render(); + + fireEvent.click(getStar(5)); + + // 9 was stored as 5, so selecting star 5 is still reported, and nothing loops + expect(handleChange).toHaveBeenCalledTimes(2); + expect(getFilledTotal()).toBe(5); + }); + + test.each([ + ['a whole value', {value: 4}], + ['a whole default value', {defaultValue: 2}], + ['read-only', {readOnly: true}], + ['read-only with a fraction', {readOnly: true, value: 2.5}], + ['disabled', {disabled: true}] + ])('should not call `onChange` for %s', (_, props) => { + const handleChange = jest.fn(); + render(); + + expect(handleChange).not.toHaveBeenCalled(); + }); + + test('should report once when a controlled parent ignores it', () => { + const handleChange = jest.fn(); + const {rerender} = render(); + rerender(); + + expect(handleChange).toHaveBeenCalledTimes(1); + }); + + test('should report again when the shown rating changes', () => { + const handleChange = jest.fn(); + const {rerender} = render(); + rerender(); + + expect(handleChange).toHaveBeenLastCalledWith({value: 5, normalized: true}); + expect(handleChange).toHaveBeenCalledTimes(2); + }); + + test('should report again after the app goes back to the same value', () => { + const handleChange = jest.fn(); + const {rerender} = render(); + rerender(); + rerender(); + + expect(handleChange).toHaveBeenCalledTimes(2); + }); + + test('should report when leaving read-only mode', () => { + const handleChange = jest.fn(); + const {rerender} = render(); + rerender(); + + expect(handleChange).toHaveBeenCalledWith({value: 3, normalized: true}); + }); + + test('should report when enabled', () => { + const handleChange = jest.fn(); + const {rerender} = render(); + rerender(); + + expect(handleChange).toHaveBeenCalledWith({value: 3, normalized: true}); + }); + + test('should not report from `StarRatingsBase`', () => { + const handleChange = jest.fn(); + render(); + + expect(handleChange).not.toHaveBeenCalled(); + }); + }); + + // While selecting, the stars up to and including the focused one are filled as a preview + describe('focus preview', () => { + const focus = (el) => act(() => el.focus()); + + const renderWithOutside = (props) => render( +
+ + +
+ ); + + test('should fill up to a higher focused star without changing the value', () => { + const handleChange = jest.fn(); + render(); + + focus(getStar(5)); + + expect(getFilledTotal()).toBe(5); + expect(handleChange).not.toHaveBeenCalled(); + }); + + test('should fill only up to a lower focused star', () => { + render(); + + focus(getStar(2)); + + expect(getFills()).toEqual(['100%', '100%', '0%', '0%', '0%']); + }); + + test('should follow focus from star to star', () => { + render(); + + focus(getStar(5)); + focus(getStar(1)); + + expect(getFilledTotal()).toBe(1); + }); + + test('should show the selected rating again when focus leaves the row', () => { + renderWithOutside({defaultValue: 3}); + + focus(getStar(5)); + focus(document.getElementById('outside')); + + expect(getFilledTotal()).toBe(3); + }); + + test('should select the previewed star on click', () => { + const handleChange = jest.fn(); + renderWithOutside({defaultValue: 3, onChange: handleChange}); + + focus(getStar(4)); + fireEvent.click(getStar(4)); + focus(document.getElementById('outside')); + + expect(handleChange).toHaveBeenCalledWith({value: 4}); + expect(getFilledTotal()).toBe(4); + }); + + test('should keep the selected star marked while previewing', () => { + render(); + + focus(getStar(5)); + + expect(getStar(3)).toHaveAttribute('data-star-selected'); + expect(getStar(5)).not.toHaveAttribute('data-star-selected'); + }); + + test('should not preview when disabled', () => { + render(); + + focus(getStar(5)); + + expect(getStar(5)).toHaveFocus(); + expect(getFilledTotal()).toBe(2); + }); + + test('should forward `onFocus` and `onBlur`', () => { + const handleFocus = jest.fn(); + const handleBlur = jest.fn(); + renderWithOutside({onBlur: handleBlur, onFocus: handleFocus}); + + focus(getStar(2)); + focus(document.getElementById('outside')); + + expect(handleFocus).toHaveBeenCalledTimes(1); + expect(handleBlur).toHaveBeenCalledTimes(1); + }); + + test('should fill up to `focusedStar` in `StarRatingsBase`', () => { + render(); + + expect(getFilledTotal()).toBe(4); + }); + + test.each([ + ['read-only', {readOnly: true, value: 2.5}, 2.5], + ['disabled', {disabled: true, value: 2}, 2] + ])('should ignore `focusedStar` when %s', (_, props, expected) => { + render(); + + expect(getFilledTotal()).toBe(expected); + }); + }); + + // Spotlight enters the row at this star, so Enter keeps the shown rating + describe('selected star', () => { + const getSelected = () => getStars().filter((star) => star.hasAttribute('data-star-selected')); + + test('should mark star 3 when no value is set', () => { + render(); + + expect(getSelected()).toEqual([getStar(3)]); + }); + + test('should mark the clamped value', () => { + render(); + + expect(getSelected()).toEqual([getStar(1)]); + }); + + test('should follow the selection', () => { + render(); + + fireEvent.click(getStar(5)); + + expect(getSelected()).toEqual([getStar(5)]); + }); + + test('should not mark a star when read-only', () => { + render(); + + expect(getSelected()).toEqual([]); + }); + }); + describe('disabled', () => { test('should not be disabled by default', () => { render(); @@ -307,6 +544,90 @@ describe('StarRatings', () => { }); }; + const touchStartOn = (star) => { + document.elementFromPoint = jest.fn(() => star); + fireEvent.touchStart(document.getElementById(ID), { + touches: [{clientX: 0, clientY: 0}] + }); + }; + + // Spec (touch drag, step 1 to 2): with 3 stars, pressing star 4 selects 4 right away + test('should select the pressed star as soon as the finger lands', () => { + const handleChange = jest.fn(); + render(); + + touchStartOn(getStar(4)); + + expect(handleChange).toHaveBeenCalledTimes(1); + expect(handleChange).toHaveBeenCalledWith({value: 4}); + expect(getFilledTotal()).toBe(4); + expect(getStar(4)).toHaveFocus(); + }); + + test('should not call `onChange` when pressing the shown star', () => { + const handleChange = jest.fn(); + render(); + handleChange.mockClear(); // the mount sync of the default 3 + + touchStartOn(getStar(3)); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getStar(3)).toHaveFocus(); + }); + + test('should keep updating while dragging after the press', () => { + const handleChange = jest.fn(); + render(); + + touchStartOn(getStar(4)); + touchMoveOver(getStar(5)); + touchMoveOver(document.body); + + expect(handleChange.mock.calls.map(([ev]) => ev.value)).toEqual([4, 5]); + expect(getFilledTotal()).toBe(5); + }); + + test('should keep the value when the press is outside the stars', () => { + const handleChange = jest.fn(); + render(); + + touchStartOn(document.body); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getFilledTotal()).toBe(2); + }); + + test.each([ + ['disabled', {disabled: true}], + ['read-only', {readOnly: true}] + ])('should not select on press when %s', (_, props) => { + const handleChange = jest.fn(); + render(); + + touchStartOn(getStar(4)); + + expect(handleChange).not.toHaveBeenCalled(); + expect(getFilledTotal()).toBe(2); + }); + + // The press already selected the star, so the click on release would call `onChange` again + test('should cancel the click that follows the touch', () => { + render(); + + touchStartOn(getStar(4)); + + expect(fireEvent.touchEnd(document.getElementById(ID))).toBe(false); + }); + + test.each([ + ['disabled', {disabled: true}], + ['read-only', {readOnly: true}] + ])('should not cancel the touch end when %s', (_, props) => { + render(); + + expect(fireEvent.touchEnd(document.getElementById(ID))).toBe(true); + }); + test('should call `onChange` with the star under the finger', () => { const handleChange = jest.fn(); render(); @@ -319,7 +640,7 @@ describe('StarRatings', () => { test('should not call `onChange` again while staying on the same star', () => { const handleChange = jest.fn(); - render(); + render(); touchMoveOver(getStar(2)); touchMoveOver(getStar(2)); @@ -329,7 +650,7 @@ describe('StarRatings', () => { test('should keep the last value when the finger leaves the row', () => { const handleChange = jest.fn(); - render(); + render(); touchMoveOver(getStar(3)); touchMoveOver(document.body); @@ -338,6 +659,26 @@ describe('StarRatings', () => { expect(getFilledTotal()).toBe(3); }); + // Regression: the star was compared with the raw `value` (undefined), not the 3 shown + test('should not call `onChange` when moving on the shown default star', () => { + const handleChange = jest.fn(); + render(); + handleChange.mockClear(); // the mount sync, covered in "shown value sync" + + touchMoveOver(getStar(3)); + + expect(handleChange).not.toHaveBeenCalled(); + }); + + test('should compare with the clamped value in `StarRatingsBase`', () => { + const handleChange = jest.fn(); + render(); + + touchMoveOver(getStar(5)); + + expect(handleChange).not.toHaveBeenCalled(); + }); + test('should not call `onChange` when disabled', () => { const handleChange = jest.fn(); render(); diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index 6067180db..2a9bdc0ea 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -12,7 +12,7 @@ export default { }; export const _StarRatings = (args) => { - const [defaultValue, setDefaultValue] = useState(0); // eslint-disable-line react-hooks/rules-of-hooks + const [defaultValue, setDefaultValue] = useState(3); // eslint-disable-line react-hooks/rules-of-hooks const handleValue = useCallback((event) => { // eslint-disable-line react-hooks/rules-of-hooks action('onChange')(event); @@ -38,6 +38,6 @@ export const _StarRatings = (args) => { boolean('disabled', _StarRatings, StarRatingsConfig); boolean('readOnly', _StarRatings, StarRatingsConfig); boolean('overlayMode', _StarRatings, StarRatingsConfig); -range('value', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}); +range('value', _StarRatings, StarRatingsConfig, {min: 0, max: 5, step: 0.01}, 0); _StarRatings.storyName = 'StarRatings'; diff --git a/tests/screenshot/apps/components/StarRatings.js b/tests/screenshot/apps/components/StarRatings.js index 3e90cc3a9..f4b05da7f 100644 --- a/tests/screenshot/apps/components/StarRatings.js +++ b/tests/screenshot/apps/components/StarRatings.js @@ -17,12 +17,19 @@ const starRatingsSmokeTests = [ ]; +// The row sizes itself to the number of stars +const starRatingsMaxTests = [ + , + , + +]; + // Every tenth of a star, to check the fill steps const starRatingsFillStepTests = [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9].map((value) => ( )); -// Focus lands on the first star. Values differ from the smoke tests so each screenshot is unique. +// Focus lands on the selected star. Values differ from the smoke tests so each screenshot is unique. const starRatingsFocusTests = [ , , @@ -42,6 +49,7 @@ const starRatingsRtlTests = [ const StarRatingsTests = [ ...starRatingsSmokeTests, + ...starRatingsMaxTests, ...starRatingsFillStepTests, ...withConfig({focus: true}, starRatingsFocusTests), ...withConfig({wrapper: {light: true}}, starRatingsOverlayModeTests), diff --git a/tests/ui/apps/StarRatings/StarRatings-View.js b/tests/ui/apps/StarRatings/StarRatings-View.js index 97ee3fb95..5a15ba030 100644 --- a/tests/ui/apps/StarRatings/StarRatings-View.js +++ b/tests/ui/apps/StarRatings/StarRatings-View.js @@ -10,9 +10,15 @@ spotlight.setPointerMode(false); const App = (props) => { const [lastChange, setLastChange] = useState('none'); + const [lastNormalized, setLastNormalized] = useState('none'); - const handleChange = useCallback(({value}) => { - setLastChange(String(value)); + // User selections and normalized values (e.g. the default 3) are shown separately + const handleChange = useCallback(({normalized, value}) => { + if (normalized) { + setLastNormalized(String(value)); + } else { + setLastChange(String(value)); + } }, []); return ( @@ -39,7 +45,16 @@ const App = (props) => { onChange={handleChange} />
+
+ +
{lastChange}
+
{lastNormalized}
); }; diff --git a/tests/ui/specs/StarRatings/StarRatings-specs.js b/tests/ui/specs/StarRatings/StarRatings-specs.js index e707f1b62..516159fa1 100644 --- a/tests/ui/specs/StarRatings/StarRatings-specs.js +++ b/tests/ui/specs/StarRatings/StarRatings-specs.js @@ -5,7 +5,8 @@ describe('StarRatings', function () { const { starRatingsDefault, starRatingsReadOnly, - starRatingsDisabled + starRatingsDisabled, + starRatingsMax3 } = Page.components; describe('LTR locale', function () { @@ -14,14 +15,19 @@ describe('StarRatings', function () { }); describe('default', function () { - it('should focus the first star on load', async function () { - expect(await starRatingsDefault.star(1).isFocused()).toBe(true); + it('should focus the selected star on load', async function () { + expect(await starRatingsDefault.star(3).isFocused()).toBe(true); }); it('should show 3 stars when no value is set', async function () { expect(await starRatingsDefault.filledStars()).toBe(3); }); + it('should report the default 3 stars to the app', async function () { + expect(await Page.lastNormalized).toBe('3'); + expect(await Page.lastChange).toBe('none'); + }); + it('should show a half star in read-only mode', async function () { expect(await starRatingsReadOnly.fills()).toEqual(['100%', '100%', '100%', '50%', '0%']); }); @@ -33,8 +39,23 @@ describe('StarRatings', function () { expect(await starRatingsDefault.isDisabled).toBe(false); }); + // The row sizes itself, so its pointer/touch area matches the stars for any `max` + it('should size the row to its stars', async function () { + const row = await starRatingsMax3.rect(); + const first = await starRatingsMax3.rect(1); + const last = await starRatingsMax3.rect(3); + + expect(Math.abs(row.left - first.left)).toBeLessThan(1); + expect(Math.abs(row.right - last.right)).toBeLessThan(1); + expect(await starRatingsMax3.star(4).isExisting()).toBe(false); + }); + + it('should make a longer row for more stars', async function () { + expect((await starRatingsDefault.rect()).width).toBeGreaterThan((await starRatingsMax3.rect()).width); + }); + it('should show the focus outline only on the focused star', async function () { - expect(await starRatingsDefault.hasFocusOutline(1)).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(3)).toBe(true); expect(await starRatingsDefault.hasFocusOutline(2)).toBe(false); }); }); @@ -43,14 +64,20 @@ describe('StarRatings', function () { it('should move focus to the next star', async function () { await Page.spotlightRight(); - expect(await starRatingsDefault.star(2).isFocused()).toBe(true); - expect(await starRatingsDefault.hasFocusOutline(2)).toBe(true); - expect(await starRatingsDefault.hasFocusOutline(1)).toBe(false); + expect(await starRatingsDefault.star(4).isFocused()).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(4)).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(3)).toBe(false); + }); + + // Spec: pressing Enter right away keeps the default 3 stars + it('should keep the shown rating when selecting right away', async function () { + await Page.spotlightSelect(); + + expect(await starRatingsDefault.filledStars()).toBe(3); + expect(await Page.lastChange).toBe('3'); }); it('should select the focused star', async function () { - await Page.spotlightRight(); - await Page.spotlightRight(); await Page.spotlightRight(); await Page.spotlightSelect(); @@ -59,19 +86,45 @@ describe('StarRatings', function () { }); it('should select a lower star', async function () { + await Page.spotlightLeft(); + await Page.spotlightLeft(); await Page.spotlightSelect(); expect(await starRatingsDefault.filledStars()).toBe(1); expect(await Page.lastChange).toBe('1'); }); - it('should not change the fill when focusing an empty star', async function () { + // Focus preview: stars up to and including the focused one fill, the value does not change + it('should fill up to a higher focused star without selecting it', async function () { + await Page.spotlightRight(); + await Page.spotlightRight(); + + expect(await starRatingsDefault.star(5).isFocused()).toBe(true); + expect(await starRatingsDefault.filledStars()).toBe(5); + expect(await Page.lastChange).toBe('none'); + }); + + it('should fill only up to a lower focused star', async function () { + await Page.spotlightLeft(); + + expect(await starRatingsDefault.fills()).toEqual(['100%', '100%', '0%', '0%', '0%']); + expect(await Page.lastChange).toBe('none'); + }); + + it('should show the selected rating again when focus leaves the row', async function () { await Page.spotlightRight(); + await Page.spotlightDown(); + + expect(await starRatingsDefault.filledStars()).toBe(3); + }); + + it('should not fill up to the focused star of a disabled rating', async function () { + await Page.spotlightDown(); await Page.spotlightRight(); await Page.spotlightRight(); - expect(await starRatingsDefault.star(4).isFocused()).toBe(true); - expect(await starRatingsDefault.fill(4)).toBe('0%'); + expect(await starRatingsDisabled.star(4).isFocused()).toBe(true); + expect(await starRatingsDisabled.filledStars()).toBe(2); }); it('should skip the read-only rating and focus the disabled one', async function () { @@ -89,15 +142,14 @@ describe('StarRatings', function () { }); it('should return to the last focused star', async function () { - await Page.spotlightRight(); - await Page.spotlightRight(); + await Page.spotlightLeft(); + await Page.spotlightLeft(); await Page.spotlightDown(); - // A rating entered for the first time starts at its first star - expect(await starRatingsDisabled.star(1).isFocused()).toBe(true); + // A rating entered for the first time starts at its selected star (defaultValue 2) + expect(await starRatingsDisabled.star(2).isFocused()).toBe(true); - // Move to the far end, so only "last focused" (not position) leads back to star 3 - await Page.spotlightRight(); + // Move to the far end, so only "last focused" (not position) leads back to star 1 await Page.spotlightRight(); await Page.spotlightRight(); await Page.spotlightRight(); @@ -106,7 +158,7 @@ describe('StarRatings', function () { await Page.spotlightUp(); - expect(await starRatingsDefault.star(3).isFocused()).toBe(true); + expect(await starRatingsDefault.star(1).isFocused()).toBe(true); }); }); @@ -144,11 +196,11 @@ describe('StarRatings', function () { }); it('should move focus to the next star with the left key', async function () { - expect(await starRatingsDefault.star(1).isFocused()).toBe(true); + expect(await starRatingsDefault.star(3).isFocused()).toBe(true); await Page.spotlightLeft(); - expect(await starRatingsDefault.star(2).isFocused()).toBe(true); + expect(await starRatingsDefault.star(4).isFocused()).toBe(true); }); it('should fill a partial star from the right', async function () { diff --git a/tests/ui/specs/StarRatings/StarRatingsPage.js b/tests/ui/specs/StarRatings/StarRatingsPage.js index 92dd38de0..7512e67d3 100644 --- a/tests/ui/specs/StarRatings/StarRatingsPage.js +++ b/tests/ui/specs/StarRatings/StarRatingsPage.js @@ -47,6 +47,15 @@ class StarRatingsInterface { async starLeft (number) { return (await this.star(number).getLocation()).x; } + + // {left, right, width} of the row, or of one of its stars + async rect (number) { + const el = number ? await this.star(number) : await this.self; + return await browser.execute((node) => { + const {left, right, width} = node.getBoundingClientRect(); + return {left, right, width}; + }, el); + } } class StarRatingsPage extends Page { @@ -56,11 +65,13 @@ class StarRatingsPage extends Page { const starRatingsDefault = new StarRatingsInterface('starRatingsDefault'); const starRatingsReadOnly = new StarRatingsInterface('starRatingsReadOnly'); const starRatingsDisabled = new StarRatingsInterface('starRatingsDisabled'); + const starRatingsMax3 = new StarRatingsInterface('starRatingsMax3'); this.components = { starRatingsDefault, starRatingsReadOnly, - starRatingsDisabled + starRatingsDisabled, + starRatingsMax3 }; } @@ -69,6 +80,11 @@ class StarRatingsPage extends Page { return $('#lastChange').getText(); } + // Value from the last `onChange` call with `normalized` set, or 'none' + get lastNormalized () { + return $('#lastNormalized').getText(); + } + get focusedId () { return browser.execute(() => document.activeElement && document.activeElement.id); } From 9ce1d6273da3363458e80867fb15a03699b6c6e2 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:49:48 +0300 Subject: [PATCH 12/17] Review fixes --- .../sampler/stories/default/StarRatings.js | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/samples/sampler/stories/default/StarRatings.js b/samples/sampler/stories/default/StarRatings.js index 2a9bdc0ea..334d2eb05 100644 --- a/samples/sampler/stories/default/StarRatings.js +++ b/samples/sampler/stories/default/StarRatings.js @@ -1,3 +1,4 @@ +import Alert from '@enact/limestone/Alert'; import {mergeComponentMetadata} from '@enact/storybook-utils'; import {action} from '@enact/storybook-utils/addons/actions'; import {boolean, range} from '@enact/storybook-utils/addons/controls'; @@ -26,13 +27,24 @@ export const _StarRatings = (args) => { value: args['readOnly'] ? args['value'] : defaultValue }; - return (<> - - ); + return ( + !args.overlayMode ? + : + + + + ); }; boolean('disabled', _StarRatings, StarRatingsConfig); From e5a6cf8e577ca76d0e2c63fb3fd33a04cca75d21 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:10:09 +0300 Subject: [PATCH 13/17] Fixes for focus --- StarRatings/StarRatings.js | 77 +++++++++--- StarRatings/tests/StarRatings-specs.js | 113 ++++++++++++++++++ .../ui/specs/StarRatings/StarRatings-specs.js | 43 +++++++ 3 files changed, 219 insertions(+), 14 deletions(-) diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index 65f67c0fc..2c794a442 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -205,11 +205,18 @@ const ShownValueDecorator = (Wrapped) => { }; /** - * Tracks which star has focus and passes its number to `StarRatingsBase` as `focusedStar`, so - * the stars up to and including the focused one are filled as a preview. + * Handles focus for the stars: * - * Only the fill changes: `value` and `onChange` are untouched until the user selects the star. - * When focus leaves the row, `focusedStar` is cleared and the selected rating shows again. + * * Preview: passes the focused star's number to `StarRatingsBase` as `focusedStar`, so the + * stars up to and including it are filled. Only the fill changes: `value` and `onChange` are + * untouched until the user selects the star. When focus leaves the row, the selected rating + * shows again. + * * Pointer: while the pointer is inside the row, focus stays on the last star that received it, + * until another star receives it. Spotlight blurs the focused element whenever the pointer is + * over something that is not spottable (the gaps between stars, or the edge of a star on some + * platforms); that blur is undone right away, before anything is painted, and is not passed + * on to `onBlur`/`onFocus`. Leaving the row blurs as usual. Pointer events are not stopped, so + * Spotlight and other listeners still see every move. * * @hoc * @param {Component} Wrapped The component to wrap. @@ -217,29 +224,70 @@ const ShownValueDecorator = (Wrapped) => { * @private */ const FocusPreviewDecorator = (Wrapped) => { - const FocusPreview = ({onBlur, onFocus, ...rest}) => { + const FocusPreview = ({onBlur, onFocus, onMouseEnter, onMouseLeave, ...rest}) => { const {id} = rest; const [focusedStar, setFocusedStar] = useState(null); + const pointerInside = useRef(false); + const restoring = useRef(false); - const handleFocus = useCallback((ev) => { - if (onFocus) onFocus(ev); + const isOwnStar = useCallback((node) => ( + Boolean(node && typeof node.id === 'string' && node.id.startsWith(`${id}-star-`)) + ), [id]); + + const handleMouseEnter = useCallback((ev) => { + pointerInside.current = true; + if (onMouseEnter) onMouseEnter(ev); + }, [onMouseEnter]); + // `mouseleave` fires before the `mousemove` Spotlight blurs on, so leaving is not undone + const handleMouseLeave = useCallback((ev) => { + pointerInside.current = false; + if (onMouseLeave) onMouseLeave(ev); + }, [onMouseLeave]); + + const handleFocus = useCallback((ev) => { // Focus events bubble from the stars; anything else in the row is ignored - const starId = ev.target && ev.target.id; - const isStar = typeof starId === 'string' && starId.startsWith(`${id}-star-`); - setFocusedStar(isStar ? getStarNumber(starId, id) || null : null); - }, [id, onFocus]); + setFocusedStar(isOwnStar(ev.target) ? getStarNumber(ev.target.id, id) || null : null); + + if (restoring.current) return; // the star got its focus back; nothing changed + if (onFocus) onFocus(ev); + }, [id, isOwnStar, onFocus]); const handleBlur = useCallback((ev) => { + const star = ev.target; + + // Blurred to nothing (not to another element) while the pointer is still inside: keep + // focus on this star. Not while the window itself loses focus. + const to = ev.relatedTarget; + const toNothing = !to || to === document || to === document.body; + if (pointerInside.current && toNothing && isOwnStar(star) && document.hasFocus()) { + restoring.current = true; + try { + focusStar(star); + } finally { + restoring.current = false; + } + if (document.activeElement === star) return; + } + if (onBlur) onBlur(ev); // Moving between stars fires blur then focus; only clear when focus leaves the row if (!ev.relatedTarget || !ev.currentTarget.contains(ev.relatedTarget)) { setFocusedStar(null); } - }, [onBlur]); + }, [isOwnStar, onBlur]); - return ; + return ( + + ); }; return FocusPreview; @@ -258,7 +306,8 @@ const FocusPreviewDecorator = (Wrapped) => { * with `onChange` when it differs from `value`, and enters the row at the selected star, so * pressing Enter or Done keeps the rating the user sees. While a star has focus, the stars * up to and including it are filled as a preview (`value` changes only on selection); the - * selected rating shows again when focus leaves the row. + * selected rating shows again when focus leaves the row. With a pointer, focus stays on the + * last hovered star while the pointer is between stars, until it leaves the row. * * `disabled` works in both modes: the rating is still shown, at 30% opacity, but cannot be * changed. `overlayMode` switches the color of the empty parts for light overlay surfaces. diff --git a/StarRatings/tests/StarRatings-specs.js b/StarRatings/tests/StarRatings-specs.js index 71a8a8452..f34d0c4eb 100644 --- a/StarRatings/tests/StarRatings-specs.js +++ b/StarRatings/tests/StarRatings-specs.js @@ -445,6 +445,119 @@ describe('StarRatings', () => { }); }); + // Spotlight blurs the focused star when the pointer is over anything that is not spottable + describe('pointer inside the row', () => { + const focus = (el) => act(() => el.focus()); + const blur = (el) => act(() => el.blur()); + const row = () => document.getElementById(ID); + + // jsdom's document never has focus; a real page does while the user interacts with it + beforeEach(() => { + jest.spyOn(document, 'hasFocus').mockReturnValue(true); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('should blur when the window loses focus', () => { + document.hasFocus.mockReturnValue(false); + render(); + fireEvent.mouseEnter(row()); + focus(getStar(2)); + + blur(getStar(2)); + + expect(getStar(2)).not.toHaveFocus(); + }); + + const renderWithOutside = (props) => render( +
+ + +
+ ); + + test('should keep focus on the last star while the pointer is inside', () => { + render(); + fireEvent.mouseEnter(row()); + focus(getStar(4)); + + blur(getStar(4)); // what Spotlight does over a gap + + expect(getStar(4)).toHaveFocus(); + expect(getFilledTotal()).toBe(4); + }); + + test('should not report the undone blur', () => { + const handleBlur = jest.fn(); + const handleFocus = jest.fn(); + render(); + fireEvent.mouseEnter(row()); + focus(getStar(2)); + + blur(getStar(2)); + + expect(handleFocus).toHaveBeenCalledTimes(1); + expect(handleBlur).not.toHaveBeenCalled(); + }); + + test('should move focus to the next star the pointer reaches', () => { + render(); + fireEvent.mouseEnter(row()); + focus(getStar(2)); + blur(getStar(2)); + + focus(getStar(5)); + + expect(getStar(5)).toHaveFocus(); + expect(getFilledTotal()).toBe(5); + }); + + test('should blur when the pointer has left the row', () => { + render(); + fireEvent.mouseEnter(row()); + focus(getStar(5)); + fireEvent.mouseLeave(row()); + + blur(getStar(5)); + + expect(getStar(5)).not.toHaveFocus(); + expect(getFilledTotal()).toBe(3); + }); + + test('should blur when the pointer never entered the row', () => { + render(); + focus(getStar(5)); + + blur(getStar(5)); + + expect(getStar(5)).not.toHaveFocus(); + }); + + test('should let focus move to another element while the pointer is inside', () => { + renderWithOutside({}); + fireEvent.mouseEnter(row()); + focus(getStar(2)); + + focus(document.getElementById('outside')); // e.g. 5-way to a button + + expect(document.getElementById('outside')).toHaveFocus(); + }); + + test('should forward `onMouseEnter` and `onMouseLeave`', () => { + const handleEnter = jest.fn(); + const handleLeave = jest.fn(); + render(); + + fireEvent.mouseEnter(row()); + fireEvent.mouseLeave(row()); + + expect(handleEnter).toHaveBeenCalledTimes(1); + expect(handleLeave).toHaveBeenCalledTimes(1); + }); + }); + // Spotlight enters the row at this star, so Enter keeps the shown rating describe('selected star', () => { const getSelected = () => getStars().filter((star) => star.hasAttribute('data-star-selected')); diff --git a/tests/ui/specs/StarRatings/StarRatings-specs.js b/tests/ui/specs/StarRatings/StarRatings-specs.js index 516159fa1..89b0f3c29 100644 --- a/tests/ui/specs/StarRatings/StarRatings-specs.js +++ b/tests/ui/specs/StarRatings/StarRatings-specs.js @@ -163,6 +163,49 @@ describe('StarRatings', function () { }); describe('pointer', function () { + it('should focus and fill up to the hovered star', async function () { + await starRatingsDefault.star(5).moveTo(); + + expect(await starRatingsDefault.star(5).isFocused()).toBe(true); + expect(await starRatingsDefault.filledStars()).toBe(5); + expect(await Page.lastChange).toBe('none'); + }); + + // Regression: the gaps are not spottable, so Spotlight used to blur the star there + it('should keep focus on the last hovered star while the pointer is between stars', async function () { + // Middle of the gap after star 2: half the distance between two stars' centers + const gap = (await starRatingsDefault.starLeft(3) - await starRatingsDefault.starLeft(2)) / 2; + + await starRatingsDefault.star(2).moveTo(); + await starRatingsDefault.star(2).moveTo({xOffset: gap}); + + expect(await starRatingsDefault.star(2).isFocused()).toBe(true); + expect(await starRatingsDefault.hasFocusOutline(2)).toBe(true); + expect(await starRatingsDefault.filledStars()).toBe(2); + }); + + it('should keep a star focused at every step from one star to another', async function () { + // Steps of a quarter of the distance between two stars' centers, from star 5 to star 1 + const pitch = await starRatingsDefault.starLeft(2) - await starRatingsDefault.starLeft(1); + const focused = []; + + for (let step = 0; step <= 16; step++) { + await starRatingsDefault.star(5).moveTo({xOffset: -step * pitch / 4}); + focused.push(await Page.focusedId); + } + + expect(focused.filter((id) => !/^starRatingsDefault-star-/.test(id || ''))).toEqual([]); + expect(focused[focused.length - 1]).toBe('starRatingsDefault-star-1'); + }); + + it('should leave the stars when the pointer leaves the row', async function () { + await starRatingsDefault.star(2).moveTo(); + await $('#lastChange').moveTo(); + + expect(await Page.focusedId).not.toMatch(/^starRatingsDefault-star-/); + expect(await starRatingsDefault.filledStars()).toBe(3); + }); + it('should select the clicked star', async function () { await starRatingsDefault.star(5).click(); From 7bb49eefa697c67c591edd9ee4b3095a3b8f4b93 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:26:42 +0300 Subject: [PATCH 14/17] Trigger Build From 6d0a916e59d6f641f3199498a74925e4833fc87c Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:01:25 +0300 Subject: [PATCH 15/17] Review fixes. --- StarRatings/StarRatings.js | 62 ++++++++++-- StarRatings/tests/StarRatings-specs.js | 94 +++++++++++++++++++ .../ui/specs/StarRatings/StarRatings-specs.js | 71 +++++++++++++- 3 files changed, 215 insertions(+), 12 deletions(-) diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index 2c794a442..ae56d947c 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -61,6 +61,15 @@ const getStarFromTouch = (ev, id) => { */ const getStarNumber = (starId, id) => Number(starId.slice(`${id}-star-`.length)); +/** + * `true` while a touch focuses a star (see `selectTouchedStar`), so that focus is not redirected + * to the selected star. + * + * @type {Boolean} + * @private + */ +let touchFocusing = false; + /** * Moves focus to a star, which shows its focus outline. * @@ -131,7 +140,12 @@ const selectTouchedStar = (ev, {id, max, onChange, readOnly, value}) => { const star = getStarFromTouch(ev, id); if (!star) return false; // outside the row: keep the last value and focus - focusStar(star); + touchFocusing = true; + try { + focusStar(star); + } finally { + touchFocusing = false; + } const shown = toTenths(value, max, readOnly) / 10; const starNumber = getStarNumber(star.id, id); @@ -142,8 +156,9 @@ const selectTouchedStar = (ev, {id, max, onChange, readOnly, value}) => { }; /** - * Selector of the selected star, which Spotlight focuses when the row is entered for the first - * time, so pressing Enter right away keeps the rating that is shown. + * Selector of the selected star, which Spotlight focuses whenever 5-way navigation enters the + * row. A star that was only focused (previewed) and not selected is not remembered, so coming + * back shows and focuses the saved rating, and pressing Enter right away keeps it. * * @type {String} * @private @@ -217,6 +232,9 @@ const ShownValueDecorator = (Wrapped) => { * platforms); that blur is undone right away, before anything is painted, and is not passed * on to `onBlur`/`onFocus`. Leaving the row blurs as usual. Pointer events are not stopped, so * Spotlight and other listeners still see every move. + * * 5-way return: when 5-way focus comes back to the row from outside, it goes to the selected + * star, even when Spotlight would restore a star that was only hovered or focused, or move from + * the pointer's last position. Only the selected star's focus is passed on to `onFocus`. * * @hoc * @param {Component} Wrapped The component to wrap. @@ -229,6 +247,7 @@ const FocusPreviewDecorator = (Wrapped) => { const [focusedStar, setFocusedStar] = useState(null); const pointerInside = useRef(false); const restoring = useRef(false); + const redirecting = useRef(false); const isOwnStar = useCallback((node) => ( Boolean(node && typeof node.id === 'string' && node.id.startsWith(`${id}-star-`)) @@ -246,6 +265,30 @@ const FocusPreviewDecorator = (Wrapped) => { }, [onMouseLeave]); const handleFocus = useCallback((ev) => { + const star = ev.target; + const fromOutside = !(ev.relatedTarget && ev.currentTarget.contains(ev.relatedTarget)); + + // 5-way focus coming back to the row from outside goes to the selected star. Spotlight + // enters the row there itself, but when nothing has focus (e.g. after the pointer left) + // a key either restores the last focused star or moves from the pointer's position, + // which can land on any star. Pointer and touch focus the star under them instead. + if ( + fromOutside && isOwnStar(star) && !restoring.current && !redirecting.current && + !pointerInside.current && !touchFocusing && !Spotlight.getPointerMode() + ) { + const selected = ev.currentTarget.querySelector(SELECTED_STAR_SELECTOR); + if (selected && selected !== star) { + redirecting.current = true; + try { + focusStar(selected); + } finally { + redirecting.current = false; + } + // The selected star's own focus event already updated the preview and onFocus + if (document.activeElement === selected) return; + } + } + // Focus events bubble from the stars; anything else in the row is ignored setFocusedStar(isOwnStar(ev.target) ? getStarNumber(ev.target.id, id) || null : null); @@ -256,6 +299,9 @@ const FocusPreviewDecorator = (Wrapped) => { const handleBlur = useCallback((ev) => { const star = ev.target; + // Leaving the star that `handleFocus` redirected from: focus never really arrived there + if (redirecting.current) return; + // Blurred to nothing (not to another element) while the pointer is still inside: keep // focus on this star. Not while the window itself loses focus. const to = ev.relatedTarget; @@ -557,8 +603,9 @@ const StarRatingsBase = kind({ * * Stores the rating itself when `value` is not set (see `defaultValue`), reports the shown * rating with `onChange` when it differs from `value` (see `onChange`), groups the stars for - * Spotlight navigation (focus enters at the selected star the first time, then returns to the - * last focused star), fills the stars up to the focused one as a preview while selecting, and + * Spotlight navigation (5-way focus always enters at the selected star, not the last focused or + * hovered one), + * fills the stars up to the focused one as a preview while selecting, and * applies the current skin. * * @hoc @@ -573,9 +620,10 @@ const StarRatingsDecorator = compose( ShownValueDecorator, FocusPreviewDecorator, SpotlightContainerDecorator({ - // Entering for the first time focuses the selected star, then the last focused one + // Always enter at the selected star: a star focused without being selected is a preview + // only, so returning to the row goes back to the saved rating defaultElement: SELECTED_STAR_SELECTOR, - enterTo: 'last-focused' + enterTo: 'default-element' }), Skinnable ); diff --git a/StarRatings/tests/StarRatings-specs.js b/StarRatings/tests/StarRatings-specs.js index f34d0c4eb..23a075aa0 100644 --- a/StarRatings/tests/StarRatings-specs.js +++ b/StarRatings/tests/StarRatings-specs.js @@ -1,6 +1,8 @@ import '@testing-library/jest-dom'; import {act, fireEvent, render} from '@testing-library/react'; +import Spotlight from '@enact/spotlight'; + import {FILL_STEPS} from '../Star'; import StarRatings, {StarRatingsBase} from '../StarRatings'; @@ -558,6 +560,98 @@ describe('StarRatings', () => { }); }); + // When nothing has focus (e.g. after the pointer left), Spotlight's first 5-way key restores + // the last focused star or moves from the pointer's position; focus goes to the selected star + describe('5-way focus returning to the row', () => { + const focus = (el) => act(() => el.focus()); + const originalElementFromPoint = document.elementFromPoint; + + beforeEach(() => { + Spotlight.setPointerMode(false); + }); + + afterEach(() => { + Spotlight.setPointerMode(true); + document.elementFromPoint = originalElementFromPoint; + }); + + test('should focus the selected star instead of another star', () => { + render(); + + focus(getStar(5)); + + expect(getStar(3)).toHaveFocus(); + expect(getFilledTotal()).toBe(3); + }); + + test('should report only the selected star\'s focus', () => { + const handleFocus = jest.fn(); + const handleBlur = jest.fn(); + render(); + + focus(getStar(5)); + + expect(handleFocus).toHaveBeenCalledTimes(1); + expect(handleFocus.mock.calls[0][0].target).toBe(getStar(3)); + expect(handleBlur).not.toHaveBeenCalled(); + }); + + test('should focus a newly selected star', () => { + render(); + fireEvent.click(getStar(5)); + + focus(getStar(1)); + + expect(getStar(5)).toHaveFocus(); + }); + + test('should focus the selected star of a disabled rating', () => { + render(); + + focus(getStar(5)); + + expect(getStar(2)).toHaveFocus(); + }); + + test('should move freely between stars inside the row', () => { + render(); + focus(getStar(3)); + + focus(getStar(5)); + + expect(getStar(5)).toHaveFocus(); + expect(getFilledTotal()).toBe(5); + }); + + test('should keep a star the pointer focuses', () => { + Spotlight.setPointerMode(true); + render(); + + focus(getStar(5)); + + expect(getStar(5)).toHaveFocus(); + }); + + test('should keep a star focused while the pointer is inside the row', () => { + render(); + fireEvent.mouseEnter(document.getElementById(ID)); + + focus(getStar(5)); + + expect(getStar(5)).toHaveFocus(); + }); + + test('should keep a star focused by touch', () => { + render(); + document.elementFromPoint = jest.fn(() => getStar(4)); + + fireEvent.touchStart(document.getElementById(ID), {touches: [{clientX: 0, clientY: 0}]}); + + expect(getStar(4)).toHaveFocus(); + expect(getFilledTotal()).toBe(4); + }); + }); + // Spotlight enters the row at this star, so Enter keeps the shown rating describe('selected star', () => { const getSelected = () => getStars().filter((star) => star.hasAttribute('data-star-selected')); diff --git a/tests/ui/specs/StarRatings/StarRatings-specs.js b/tests/ui/specs/StarRatings/StarRatings-specs.js index 89b0f3c29..2e219313e 100644 --- a/tests/ui/specs/StarRatings/StarRatings-specs.js +++ b/tests/ui/specs/StarRatings/StarRatings-specs.js @@ -141,24 +141,51 @@ describe('StarRatings', function () { expect(await Page.lastChange).toBe('none'); }); - it('should return to the last focused star', async function () { + it('should enter a rating at its selected star', async function () { await Page.spotlightLeft(); await Page.spotlightLeft(); await Page.spotlightDown(); - // A rating entered for the first time starts at its selected star (defaultValue 2) + // defaultValue 2, not star 1 below the focused star expect(await starRatingsDisabled.star(2).isFocused()).toBe(true); + }); - // Move to the far end, so only "last focused" (not position) leads back to star 1 + // Default 3, 5-way to star 5 without selecting, leave the row and come back: the saved + // rating (3) is focused again, not the previewed star 5 + it('should return to the selected star when the focused star was not selected', async function () { + await Page.spotlightRight(); await Page.spotlightRight(); + + expect(await starRatingsDefault.star(5).isFocused()).toBe(true); + + await Page.spotlightDown(); + await Page.spotlightUp(); + + expect(await starRatingsDefault.star(3).isFocused()).toBe(true); + expect(await starRatingsDefault.filledStars()).toBe(3); + expect(await Page.lastChange).toBe('none'); + }); + + it('should return to a newly selected star', async function () { await Page.spotlightRight(); await Page.spotlightRight(); + await Page.spotlightSelect(); + await Page.spotlightLeft(); + await Page.spotlightDown(); + await Page.spotlightUp(); - expect(await starRatingsDisabled.star(5).isFocused()).toBe(true); + expect(await starRatingsDefault.star(5).isFocused()).toBe(true); + expect(await starRatingsDefault.filledStars()).toBe(5); + }); + it('should return to the selected star of a disabled rating', async function () { + await Page.spotlightDown(); + await Page.spotlightRight(); + await Page.spotlightRight(); await Page.spotlightUp(); + await Page.spotlightDown(); - expect(await starRatingsDefault.star(1).isFocused()).toBe(true); + expect(await starRatingsDisabled.star(2).isFocused()).toBe(true); }); }); @@ -206,6 +233,40 @@ describe('StarRatings', function () { expect(await starRatingsDefault.filledStars()).toBe(3); }); + // Hybrid: hover a star (preview only), move the pointer away, then use 5-way. Focus + // returns to the saved rating, not the hovered star Spotlight remembers. + it('should return to the selected star after hovering another one', async function () { + await starRatingsDefault.star(5).moveTo(); + await $('#lastChange').moveTo(); + + expect(await Page.focusedId).not.toMatch(/^starRatingsDefault-star-/); + + await Page.spotlightRight(); + + expect(await starRatingsDefault.star(3).isFocused()).toBe(true); + expect(await starRatingsDefault.filledStars()).toBe(3); + + await Page.spotlightSelect(); + + expect(await Page.lastChange).toBe('3'); + }); + + it('should enter a rating at its selected star when 5-way starts from the pointer', async function () { + // Up from the pointer's position below the ratings reaches the disabled one first + await starRatingsDisabled.star(5).moveTo(); + await $('#lastChange').moveTo(); + await Page.spotlightUp(); + + expect(await starRatingsDisabled.star(2).isFocused()).toBe(true); + }); + + it('should continue from the hovered star when 5-way is used without leaving', async function () { + await starRatingsDefault.star(5).moveTo(); + await Page.spotlightLeft(); + + expect(await starRatingsDefault.star(4).isFocused()).toBe(true); + }); + it('should select the clicked star', async function () { await starRatingsDefault.star(5).click(); From 117b1c8e03b1f9f0d0845ef99ad723efa37c305b Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:24:37 +0300 Subject: [PATCH 16/17] Update CHANGELOG.md Co-authored-by: Daniel Stoian <63335068+daniel-stoian-lgp@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e569c4f7a..2a1960783 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The following is a curated list of changes in the Enact limestone module, newest ### Added -- new component `StarRatings` +- `limestone/StarRatings` to display a star rating or let the user select one - `limestone/VerticalMarquee` to vertically scroll overflowing multi-line text via `ui/Marquee.MarqueeDecorator` configured with `orientation: 'vertical'` ### Changed From 5a8578169df20d8edcebc9920a90cdef622fd7b1 Mon Sep 17 00:00:00 2001 From: Daniel Stoian <63335068+daniel-stoian-lgp@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:27:18 +0300 Subject: [PATCH 17/17] Update StarRatings/StarRatings.js --- StarRatings/StarRatings.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/StarRatings/StarRatings.js b/StarRatings/StarRatings.js index ae56d947c..45cb4ec66 100644 --- a/StarRatings/StarRatings.js +++ b/StarRatings/StarRatings.js @@ -604,8 +604,7 @@ const StarRatingsBase = kind({ * Stores the rating itself when `value` is not set (see `defaultValue`), reports the shown * rating with `onChange` when it differs from `value` (see `onChange`), groups the stars for * Spotlight navigation (5-way focus always enters at the selected star, not the last focused or - * hovered one), - * fills the stars up to the focused one as a preview while selecting, and + * hovered one), fills the stars up to the focused one as a preview while selecting, and * applies the current skin. * * @hoc