From 09ad693565a8e1520d8b9246252c90071db871bf Mon Sep 17 00:00:00 2001 From: Ayshe Dzhindzhi Date: Fri, 29 May 2026 18:06:13 +0300 Subject: [PATCH 1/3] fix: make Tips cards navigable through keyboard --- .../scratch-gui/src/components/cards/card.css | 6 ++++ .../src/components/cards/cards.jsx | 36 +++++++++++++------ .../__snapshots__/cards.test.jsx.snap | 24 ++++++------- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/packages/scratch-gui/src/components/cards/card.css b/packages/scratch-gui/src/components/cards/card.css index a9e33a1caef..c0ec3c764bc 100644 --- a/packages/scratch-gui/src/components/cards/card.css +++ b/packages/scratch-gui/src/components/cards/card.css @@ -64,6 +64,7 @@ body .card-container { height: 44px; width: 44px; border-radius: 100%; + border: none; display: flex; justify-content: center; align-items: center; @@ -112,6 +113,11 @@ body .card-container { font-weight: bold; } +.header-buttons button { + background: none; + border: none; +} + .header-buttons-hidden { border-bottom: 0px; } diff --git a/packages/scratch-gui/src/components/cards/cards.jsx b/packages/scratch-gui/src/components/cards/cards.jsx index 4f99b406340..cb20e0c6912 100644 --- a/packages/scratch-gui/src/components/cards/cards.jsx +++ b/packages/scratch-gui/src/components/cards/cards.jsx @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, {Fragment} from 'react'; +import React, {Fragment, useEffect} from 'react'; import classNames from 'classnames'; import {FormattedMessage} from 'react-intl'; import Draggable from 'react-draggable'; @@ -18,9 +18,11 @@ import closeIcon from './icon--close.svg'; import {translateVideo} from '../../lib/libraries/decks/translate-video.js'; import {translateImage} from '../../lib/libraries/decks/translate-image.js'; +import {KEY} from '../../lib/navigation-keys.js'; + const CardHeader = ({onCloseCards, onShrinkExpandCards, onShowAll, totalSteps, step, expanded}) => (
-
@@ -33,7 +35,7 @@ const CardHeader = ({onCloseCards, onShrinkExpandCards, onShowAll, totalSteps, s description="Title for button to return to tutorials library" id="gui.cards.all-tutorials" /> -
+ {totalSteps > 1 ? (
{Array(totalSteps).fill(0) @@ -46,7 +48,7 @@ const CardHeader = ({onCloseCards, onShrinkExpandCards, onShowAll, totalSteps, s
) : null}
-
@@ -66,8 +68,8 @@ const CardHeader = ({onCloseCards, onShrinkExpandCards, onShowAll, totalSteps, s id="gui.cards.expand" /> } -
-
+
+
); @@ -192,7 +194,7 @@ const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => ( {onNextStep ? (
-
@@ -200,13 +202,13 @@ const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => ( draggable={false} src={isRtl ? leftArrow : rightArrow} /> -
+
) : null} {onPrevStep ? (
-
@@ -214,7 +216,7 @@ const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => ( draggable={false} src={isRtl ? rightArrow : leftArrow} /> -
+
) : null} @@ -370,6 +372,18 @@ const Cards = props => { } = props; let {x, y} = posProps; + useEffect(() => { + const handleKeyDown = e => { + if (e.key === KEY.ESCAPE) { + onCloseCards(); + } + }; + + window.addEventListener('keydown', handleKeyDown); + + return () => window.removeEventListener('keydown', handleKeyDown); + }, [onCloseCards]); + if (activeDeckId === null) return; // Tutorial cards need to calculate their own dragging bounds diff --git a/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap b/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap index 3c8fa96bd8d..f0bd00d34cf 100644 --- a/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap +++ b/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap @@ -10,26 +10,26 @@ exports[`Cards component showVideos=false shows the title image/name instead of >
-
+
+
-
+
-
+ +
+
@@ -58,26 +58,26 @@ exports[`Cards component showVideos=true shows the video step 1`] = ` >
-
+
+
-
+
-
+ +
+
From 36b90298c33d5e67b9b5f8991d32af8fe30ec0e2 Mon Sep 17 00:00:00 2001 From: Ayshe Dzhindzhi Date: Mon, 1 Jun 2026 13:43:50 +0300 Subject: [PATCH 2/3] fix: address review comments --- .../src/components/cards/cards.jsx | 78 ++++++++++++++++--- .../__snapshots__/cards.test.jsx.snap | 6 ++ 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/packages/scratch-gui/src/components/cards/cards.jsx b/packages/scratch-gui/src/components/cards/cards.jsx index cb20e0c6912..9eb963fca7b 100644 --- a/packages/scratch-gui/src/components/cards/cards.jsx +++ b/packages/scratch-gui/src/components/cards/cards.jsx @@ -1,7 +1,7 @@ import PropTypes from 'prop-types'; import React, {Fragment, useEffect} from 'react'; import classNames from 'classnames'; -import {FormattedMessage} from 'react-intl'; +import {FormattedMessage, defineMessages, useIntl} from 'react-intl'; import Draggable from 'react-draggable'; import styles from './card.css'; @@ -20,8 +20,53 @@ import {translateImage} from '../../lib/libraries/decks/translate-image.js'; import {KEY} from '../../lib/navigation-keys.js'; -const CardHeader = ({onCloseCards, onShrinkExpandCards, onShowAll, totalSteps, step, expanded}) => ( -
+const labelMap = defineMessages({ + tutorialsIcon: { + id: 'gui.cards.tutorialsIcon', + defaultMessage: 'Tutorials icon', + description: 'Label for help icon' + }, + shrinkIcon: { + id: 'gui.cards.shrinkIcon', + defaultMessage: 'Shrink icon', + description: 'Title for button to shrink how-to card' + }, + expandIcon: { + id: 'gui.cards.expandIcon', + defaultMessage: 'Expand icon', + description: 'Title for button to expand how-to card' + }, + closeIcon: { + id: 'gui.cards.closeIcon', + defaultMessage: 'Close icon', + description: 'Title for button to close how-to card' + }, + leftArrowIcon: { + id: 'gui.cards.leftArrowIcon', + defaultMessage: 'Left arrow icon', + description: 'Title for button to go to previous step in how-to card' + }, + rightArrowIcon: { + id: 'gui.cards.rightArrowIcon', + defaultMessage: 'Right arrow icon', + description: 'Title for button to go to next step in how-to card' + }, + previousStepButton: { + id: 'gui.cards.previousStepButton', + defaultMessage: 'Previous step', + description: 'Title for button to go to previous step in how-to card' + }, + nextStepButton: { + id: 'gui.cards.nextStepButton', + defaultMessage: 'Next step', + description: 'Title for button to go to next step in how-to card' + } +}); + +const CardHeader = ({onCloseCards, onShrinkExpandCards, onShowAll, totalSteps, step, expanded}) => { + const intl = useIntl(); + + return
-
-); +
; +} class VideoStep extends React.Component { @@ -189,18 +237,22 @@ ImageStep.propTypes = { title: PropTypes.node.isRequired }; -const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => ( - +const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => { + const intl = useIntl(); + + return {onNextStep ? (
@@ -211,16 +263,18 @@ const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => (
) : null} -
-); +
; +} NextPrevButtons.propTypes = { expanded: PropTypes.bool.isRequired, @@ -373,8 +427,12 @@ const Cards = props => { let {x, y} = posProps; useEffect(() => { + if (activeDeckId === null) return; + const handleKeyDown = e => { if (e.key === KEY.ESCAPE) { + e.preventDefault(); + onCloseCards(); } }; @@ -382,7 +440,7 @@ const Cards = props => { window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); - }, [onCloseCards]); + }, [onCloseCards, activeDeckId]); if (activeDeckId === null) return; diff --git a/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap b/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap index f0bd00d34cf..f8af19d28c8 100644 --- a/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap +++ b/packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap @@ -12,6 +12,7 @@ exports[`Cards component showVideos=false shows the title image/name instead of
-
; -} +
); +}; class VideoStep extends React.Component { @@ -238,9 +240,9 @@ ImageStep.propTypes = { }; const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => { - const intl = useIntl(); + const intl = useIntl(); - return + return ( {onNextStep ? (
@@ -252,7 +254,9 @@ const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => { {isRtl
@@ -268,13 +272,16 @@ const NextPrevButtons = ({isRtl, onNextStep, onPrevStep, expanded}) => { {isRtl
) : null} -
; -} +
); +}; NextPrevButtons.propTypes = { expanded: PropTypes.bool.isRequired,