Skip to content

[UEPR-518] Implement in-editor Manual Save Project Thumbnail logic #471

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
kbangelov wants to merge 23 commits into
base: develop
Choose a base branch
from
Open

[UEPR-518] Implement in-editor Manual Save Project Thumbnail logic #471

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a8dd014
chore: initial changes
kbangelov Feb 27, 2026
a193715
Merge branch 'develop' into task/uepr-518-manual-thumbnail-project-sa…
kbangelov Mar 12, 2026
45c2935
chore: moved button in editor and added modal
kbangelov Mar 12, 2026
84d2901
chore: added alerts
kbangelov Mar 13, 2026
942bb59
chore: updated focus, restricted access to button to owner
kbangelov Mar 13, 2026
42dedfc
chore: reverted some changes
kbangelov Mar 13, 2026
d858e2c
chore: fixed undefined prop
kbangelov Mar 13, 2026
cc1264b
chore: tooltip changes and more
kbangelov Mar 18, 2026
4c0d824
chore: some UI and UX improvements
kbangelov Mar 18, 2026
f4698f8
chore: addressed some comments
kbangelov Mar 19, 2026
6c06180
chore: addressed comments
kbangelov Mar 19, 2026
8934776
chore: addressed copilot comments
kbangelov Mar 19, 2026
725e94e
chore: move method into lib folder
kbangelov Mar 19, 2026
c3addb3
chore: omit unused logic
kbangelov Mar 19, 2026
4780722
chore: addressed comments
kbangelov Mar 24, 2026
bb35c3e
chore: merged components to use popup with arrow common logic
kbangelov Mar 25, 2026
5d39186
chore: remove async logic
kbangelov Mar 25, 2026
26759dd
chore: addressed comments, renamed component more appropriately
kbangelov Mar 25, 2026
02c594c
chore: refactored delete confirmation prompt and other code
kbangelov Mar 26, 2026
68ac1ed
chore: modal with arrow refactoring
kbangelov Mar 26, 2026
e792842
chore: made delete confirmation prompt use confirmation prompt
kbangelov Mar 27, 2026
b8cc0c3
chore: refactored props
kbangelov Mar 27, 2026
6356ca7
chore: removed unnecessary code
kbangelov Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions packages/scratch-gui/src/components/alerts/alert.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
body .alert {
width: 100%;
display: flex;
box-sizing: border-box;
flex-direction: row;
overflow: hidden;
justify-content: flex-start;
Expand All @@ -19,8 +20,8 @@ body .alert {
}

.alert.warn {
background: #FFF0DF;
border: 1px solid #FF8C1A;
background: $ui-alert-orange;
border: 1px solid $data-primary;
box-shadow: 0px 0px 0px 2px rgba(255, 140, 26, 0.25);
}

Expand All @@ -30,6 +31,11 @@ body .alert {
box-shadow: 0px 0px 0px 2px $extensions-light;
}

.alert.info-blue {
border: 1px solid $motion-primary;
background: $ui-primary;
}

.alert-spinner {
self-align: center;
}
Expand Down Expand Up @@ -76,7 +82,7 @@ body .alert {
width: 6.5rem;
padding: 0.55rem 0.9rem;
border-radius: 0.35rem;
background: #FF8C1A;
background: $data-primary;
color: white;
font-weight: 700;
font-size: 0.77rem;
Expand Down
8 changes: 7 additions & 1 deletion packages/scratch-gui/src/components/button/button.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const ButtonComponent = ({
iconSrc,
onClick,
children,
componentRef,
...props
}) => {

Expand All @@ -33,6 +34,7 @@ const ButtonComponent = ({
className
)}
onClick={onClick}
ref={componentRef}
{...props}
>
{icon}
Expand All @@ -47,7 +49,11 @@ ButtonComponent.propTypes = {
disabled: PropTypes.bool,
iconClassName: PropTypes.string,
iconSrc: PropTypes.string,
onClick: PropTypes.func
onClick: PropTypes.func,
componentRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({current: PropTypes.instanceOf(Element)})
])
};

export default ButtonComponent;
74 changes: 74 additions & 0 deletions packages/scratch-gui/src/components/confirmation-prompt/confirmation-prompt.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
@import "../../css/colors.css";
@import "../../css/units.css";

.modal-container {
display: flex;
width: 100%;
padding: 0.75rem;
flex-direction: column;
align-items: flex-start;
border-radius: 0.5rem;
background: $looks-secondary;
gap: 0.9375rem;
box-sizing: border-box;
}

.label {
align-self: stretch;
color: $ui-white;
text-align: center;
font-size: 1rem;
font-weight: 700;
line-height: 1.5rem;
}

.button-row {
font-weight: 700;
display: flex;
gap: 0.5rem;
width: 100%;
}

.confirm-button,
.cancel-button {
all: unset;
display: flex;
padding: 0.5rem 1rem;
justify-content: center;
align-items: center;
gap: 0.5rem;
flex: 1;
border-radius: 2.5rem;
background: inherit;
cursor: pointer;
color: $ui-white;
}

.confirm-button:focus,
.cancel-button:focus {
outline: auto;
Copy link
Copy Markdown
Contributor

@KManolov3 KManolov3 Mar 20, 2026

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Copy Markdown
Contributor Author

@kbangelov kbangelov Mar 24, 2026

Choose a reason for hiding this comment

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

currently yes because of the earlier all: unset; on the button

Copy link
Copy Markdown
Contributor

@adzhindzhi adzhindzhi Mar 24, 2026

Choose a reason for hiding this comment

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

I wonder if that's a sign not to use all:unset but rather change what actually needs to be changed? My concern is that we may unset some default button styles that we haven't though of testing. I don't have a strong preference though, that's just a thought.

}

.confirm-button span,
.cancel-button span {
font-size: 0.75rem;
font-weight: 700;
line-height: 1rem;
}

.confirm-button {
background: $ui-white;
}

.confirm-button span {
color: $looks-secondary;
}

.cancel-button {
border: 0.0625rem solid $ui-white;
}

.button-icon {
height: 1.5rem;
width: 1.5rem;
}
184 changes: 184 additions & 0 deletions packages/scratch-gui/src/components/confirmation-prompt/confirmation-prompt.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import React from 'react';
import PropTypes from 'prop-types';
import {defineMessages, FormattedMessage} from 'react-intl';

import Box from '../box/box.jsx';
import ModalWithArrow from '../modal-with-arrow/modal-with-arrow.jsx';

import arrowDownIcon from './icon--arrow-down.svg';
import arrowUpIcon from './icon--arrow-up.svg';
import arrowLeftIcon from './icon--arrow-left.svg';
import arrowRightIcon from './icon--arrow-right.svg';

import styles from './confirmation-prompt.css';
import {PopupAlign, PopupSide} from '../../lib/calculatePopupPosition.js';
import classNames from 'classnames';

const messages = defineMessages({
defaultConfirmLabel: {
defaultMessage: 'yes',
description: 'Label for confirm button in confirmation prompt',
id: 'gui.confirmationPrompt.confirm'
},
defaultCancelLabel: {
defaultMessage: 'no',
description: 'Label for cancel button in confirmation prompt',
id: 'gui.confirmationPrompt.cancel'
}
});

const defaultConfig = {
modalWidth: 200,
spaceForArrow: 16,
counterOffset: 7,
arrowOffsetFromBottom: 2,
arrowWidth: 29,
arrowHeight: 13
};

const arrowConfig = {
arrowDownIcon,
arrowUpIcon,
arrowLeftIcon,
arrowRightIcon
};

const BUTTON_ORDER = {
Copy link
Copy Markdown
Contributor

@KManolov3 KManolov3 Mar 27, 2026

Choose a reason for hiding this comment

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

Ideally, I'd generalize the props to:

`mainButtonIcon`
`mainButtonStyles`
`mainButtonLabel`
`mainButtonAction`
`secondaryButtonIcon`
`secondaryButtonStyles`
`secondaryButtonLabel`
`secondaryButtonAction`

But I'm not insisting us do that here, as the confirm/cancel separation fits the current cases

CANCEL_FIRST: 'cancelFirst',
CONFIRM_FIRST: 'confirmFirst'
};

const ConfirmationPrompt = ({
Copy link
Copy Markdown
Contributor

@KManolov3 KManolov3 Mar 18, 2026

Choose a reason for hiding this comment

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

Can we:

  • pass the positioning params (width, arrow sizes, ..) as a configuration object to the ConfirmationPrompt
  • reuse this component for the delete confirmation prompt, since it already contains a less complex version of the logic used here

Copy link
Copy Markdown
Contributor

@adzhindzhi adzhindzhi Mar 26, 2026

Choose a reason for hiding this comment

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

What I find slightly confusing about the current implementation is the following structure:

  • PopupWithArrow is a generic component used by ConfirmationPrompt, DeleteConfirmationPrompt, and FeatureCallout.
  • ConfirmationPrompt is also a generic component, but it's currently only used for the thumbnail confirmation prompt and not for the delete confirmation prompt.

I think what we actually need is a clearer separation of responsibilities:

  • SetTooltipConfirmationPrompt should use ConfirmationPrompt and pass in specific content such as title (if needed), description, className, layout, etc.
  • DeleteConfirmationPrompt should also use ConfirmationPrompt and provide its own specific content.
  • ConfirmationPrompt should use PopupWithArrow, which handles alignment and positioning.
  • FeatureCallout should use PopupWithArrow for alignment and positioning.
  • PopupWithArrow should use ReactModal to leverage its built-in behavior for handling outside clicks, escape key presses, etc.

This structure would simplify the overall design and ensure that each component has a clear, single responsibility. We may still need to refine the naming to better reflect each component's purpose, but this is the general direction I think we should take.

title,
message,
confirmLabel,
cancelLabel,
confirmIcon,
Copy link
Copy Markdown
Contributor

@KManolov3 KManolov3 Mar 27, 2026

Choose a reason for hiding this comment

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

Can we group the related props in a confirmButtonConfig, canceButtonConfig objects?

Copy link
Copy Markdown
Contributor

@adzhindzhi adzhindzhi Mar 27, 2026

Choose a reason for hiding this comment

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

Or alternatively, we can pass primaryButtonContent/secondaryButtonContent - a React node, which can either be the text if that's all we want to display, or text + icon. Both seem fine to me, whatever you feel is better.

cancelIcon,
buttonOrder = BUTTON_ORDER.CANCEL_FIRST,
onConfirm,
onCancel,
isOpen,
relativeElementRef,
side,
align,
layoutConfig,
containerClassName,
messageClassName,
confirmButtonClassName,
cancelButtonClassName
}) => {
const {
modalWidth,
spaceForArrow,
counterOffset,
arrowOffsetFromBottom,
arrowHeight,
arrowWidth
} = {...defaultConfig, ...layoutConfig};

const memoizedLayoutConfig = React.useMemo(() => ({
modalWidth,
spaceForArrow,
counterOffset,
arrowOffsetFromBottom,
arrowHeight,
arrowWidth
}), [modalWidth,
spaceForArrow,
counterOffset,
arrowOffsetFromBottom,
arrowHeight,
arrowWidth
]);

const cancelButton = (
<button
onClick={onCancel}
className={classNames(styles.cancelButton, cancelButtonClassName)}
>
{cancelIcon && (
<img
className={styles.buttonIcon}
src={cancelIcon}
aria-hidden="true"
alt=""
/>
)}
{cancelLabel ?? <FormattedMessage {...messages.defaultCancelLabel} />}
</button>
);

const confirmButton = (
<button
onClick={onConfirm}
className={classNames(styles.confirmButton, confirmButtonClassName)}
>
{confirmIcon && (
<img
className={styles.buttonIcon}
src={confirmIcon}
aria-hidden="true"
alt=""
Copy link
Copy Markdown
Contributor

@adzhindzhi adzhindzhi Mar 27, 2026

Choose a reason for hiding this comment

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

nitpick: Maybe we shouldn't leave the alt text empty, since we are also working on making the editor more accessible? Same for the cancel button?

Copy link
Copy Markdown
Contributor Author

@kbangelov kbangelov Mar 27, 2026

Choose a reason for hiding this comment

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

Since the images are not supposed to be announced, I'll just remove the alt props alltogether.

/>
)}
{confirmLabel ?? <FormattedMessage {...messages.defaultConfirmLabel} />}
</button>
);

const buttons = buttonOrder === BUTTON_ORDER.CONFIRM_FIRST ?
[confirmButton, cancelButton] : [cancelButton, confirmButton];

return (
<ModalWithArrow
isOpen={isOpen}
relativeElementRef={relativeElementRef}
onRequestClose={onCancel}
side={side}
align={align}
layoutConfig={memoizedLayoutConfig}
arrowConfig={arrowConfig}
title={title}
>
<Box className={classNames(styles.modalContainer, containerClassName)}>
<Box className={classNames(styles.label, messageClassName)}>
{message}
</Box>
<Box className={styles.buttonRow}>
{buttons}
</Box>
</Box>
</ModalWithArrow>
);
};

ConfirmationPrompt.propTypes = {
isOpen: PropTypes.bool.isRequired,
title: PropTypes.string,
message: PropTypes.node.isRequired,
confirmLabel: PropTypes.node,
cancelLabel: PropTypes.node,
confirmIcon: PropTypes.string,
cancelIcon: PropTypes.string,
buttonOrder: PropTypes.oneOf(Object.values(BUTTON_ORDER)),
onConfirm: PropTypes.func.isRequired,
onCancel: PropTypes.func.isRequired,
relativeElementRef: PropTypes.shape({current: PropTypes.instanceOf(Element)}),
side: PropTypes.oneOf(Object.values(PopupSide)).isRequired,
align: PropTypes.oneOf(Object.values(PopupAlign)),
layoutConfig: PropTypes.shape({
modalWidth: PropTypes.number,
spaceForArrow: PropTypes.number,
arrowOffsetFromBottom: PropTypes.number,
counterOffset: PropTypes.number,
arrowHeight: PropTypes.number,
arrowWidth: PropTypes.number
}),
containerClassName: PropTypes.string,
messageClassName: PropTypes.string,
confirmButtonClassName: PropTypes.string,
cancelButtonClassName: PropTypes.string
};

export {BUTTON_ORDER};
export default ConfirmationPrompt;
3 changes: 3 additions & 0 deletions packages/scratch-gui/src/components/confirmation-prompt/icon--arrow-down.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
0 ...-confirmation-prompt/icon--arrow-left.svg → .../confirmation-prompt/icon--arrow-left.svg
View file
Open in desktop
File renamed without changes
0 ...confirmation-prompt/icon--arrow-right.svg → ...confirmation-prompt/icon--arrow-right.svg
View file
Open in desktop
File renamed without changes
13 changes: 13 additions & 0 deletions packages/scratch-gui/src/components/confirmation-prompt/icon--arrow-up.svg
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@KManolov3 KManolov3 Mar 16, 2026

Choose a reason for hiding this comment

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

Why is this one bigger than the others? What method did you use for exporting them from Figma?

Copy link
Copy Markdown
Contributor Author

@kbangelov kbangelov Mar 19, 2026

Choose a reason for hiding this comment

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

There was some gap on the originally downloaded one. I wonder if it's worth resolving, since it has the same proportions and same length in the end.

Copy link
Copy Markdown
Contributor

@adzhindzhi adzhindzhi Mar 25, 2026

Choose a reason for hiding this comment

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

I think it might be, at least size-wise the stored SVG will be smaller. Though I'm not sure if the difference is significant enough to make a difference.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading