Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Font Library: No message appears when pressing the Update button #54601

Closed
t-hamano opened this issue Sep 19, 2023 · 5 comments · Fixed by #64030
Closed

Font Library: No message appears when pressing the Update button #54601

t-hamano opened this issue Sep 19, 2023 · 5 comments · Fixed by #64030
Assignees
Labels
[Feature] Font Library [Feature] Typography Font and typography-related issues and PRs [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@t-hamano
Copy link
Contributor

What problem does this address?

In the font library, for example, when you change the variation you use, you can click the Update button. When you click that button, the state is indeed saved, but no message is displayed.

What is your proposed solution?

In my opinion, there is a solution to either of the following:

  • Use the Snackbar component to notify that settings have been properly updated.
  • Remove Update button.

Personally, I prefer the latter approach. As I understand it, the behavior of the Update button has the same role as the Save button in the Global Styles. Also, the font variation setting is part of the global style.

I think whether or not to save changes made in global styles should be centralized in one button.

Screencast

7265fc7a03ca0dd6b4c87ee70be7c856.mp4
@t-hamano t-hamano added [Type] Enhancement A suggestion for improvement. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Fonts API labels Sep 19, 2023
@madhusudhand
Copy link
Member

madhusudhand commented Sep 20, 2023

@jasmussen Can you share your ideas on the messaging UI?

@jasmussen
Copy link
Contributor

jasmussen commented Sep 20, 2023

Yes, I touched on that in this PR review. I'm suggesting there do either of the following. Either when you press Update, the dialog is immediately closed, and the multi entity saving interface is opened, this one:

multi entity saving

Or, when the Update button is pressed, you are taken to your library tab in the dialog. After all, you just enqueued it to be in your library.

library

I think we should avoid snackbars in this iteration, the basic flow should be in place before we add notices to explain.

@annezazu annezazu added [Feature] Typography Font and typography-related issues and PRs and removed [Feature] Fonts API labels Sep 25, 2023
@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed [Type] Enhancement A suggestion for improvement. labels Jan 30, 2024
@afercia
Copy link
Contributor

afercia commented Jan 30, 2024

Raising this issue to bug and adding the Accessibility label, as the lack of visible and audible feedback isn't ideal.

@akasunil
Copy link
Member

A simple notice could work, in my opinion. We also use notice for font installation. Notice text can be something like Font settings were updated successfully

image

@t-hamano
Copy link
Contributor Author

To ideally solve this problem, I think the first step would be to manage the notification as a state instead of context. The notification is only active on that screen and is supposed to be unmounted when the screen changes.

Currently, The notification is managed as a context, and notification status is preserved even when switching screens, so there is redundant processing to reset the notification status (example).

I will also address this issue as part of #58428.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Font Library [Feature] Typography Font and typography-related issues and PRs [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants