Directory

Block Theme Preview: Display the theme name on the activate button by arthur791004 · Pull Request #55752 · WordPress/gutenberg · GitHub
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

Block Theme Preview: Display the theme name on the activate button #55752

Conversation

arthur791004
Copy link
Contributor

@arthur791004 arthur791004 commented Nov 1, 2023

What?

We want to display the name of the previewing theme on the “Activate” button to make it clearer what theme you're previewing.

Note that I keep the text of the “Activate” button on both “Save modal” and “Save panel” unchanged as

  • We mentioned the theme name in the description
  • The theme name might be too long on the “Save panel”

Does it make sense? Or do we still want to display the theme name there?

Why?

Resolve #52772

How?

Testing Instructions

  1. Go to Appearance > Themes
  2. Hover on a block theme
  3. Click the ”Live Preview“ button
  4. Verify the “Activate” button includes the theme name
  5. Go to Edit mode
  6. Verify the “Activate” button at the top-right corner includes the theme name as well

Testing Instructions for Keyboard

Screenshots or screencast

image

image

Copy link
Contributor

@okmttdhr okmttdhr left a comment

Choose a reason for hiding this comment

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

Tested and looked good to me! 💯 Thanks for this improvement!

isDirty: dirtyEntityRecords.length > 0,
isSaving:
dirtyEntityRecords.some( ( record ) =>
isSavingEntityRecord( record.kind, record.name, record.key )
Copy link
Contributor

@okmttdhr okmttdhr Nov 6, 2023

Choose a reason for hiding this comment

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

There seems to be a prettier error on this line.
https://github.com/WordPress/gutenberg/actions/runs/6730275641/job/18292658610?pr=55752#step:5:592

 Replace `·record.kind,·record.name,·record.key·` with `⏎↹↹↹↹↹↹↹record.kind,⏎↹↹↹↹↹↹↹record.name,⏎↹↹↹↹↹↹↹record.key⏎↹↹↹↹↹↹`  prettier/prettier

I don't think this has anything to do with this PR change, but it would be good to address it before merging! @arthur791004
I'm sorry for my late review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@arthur791004 arthur791004 force-pushed the feat/add-previewing-theme-to-activate-button branch from 6e32c7e to 0f46f65 Compare November 15, 2023 08:27
Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

I've not tested this but I approve of the direction

@okmttdhr okmttdhr merged commit 1e29bb5 into WordPress:trunk Nov 21, 2023
48 checks passed
@github-actions github-actions bot added this to the Gutenberg 17.2 milestone Nov 21, 2023
@arthur791004 arthur791004 deleted the feat/add-previewing-theme-to-activate-button branch November 21, 2023 06:32
Comment on lines +63 to +69
return sprintf( 'Activating %s', previewingThemeName );
} else if ( disabled ) {
return __( 'Saved' );
} else if ( isDirty ) {
return __( 'Activate & Save' );
return sprintf( 'Activate %s & Save', previewingThemeName );
}
return __( 'Activate' );
return sprintf( 'Activate %s', previewingThemeName );
Copy link
Contributor

Choose a reason for hiding this comment

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

With this PR, these strings are no longer translated. This needs to be fixed, @arthur791004.

Since the new strings involve interpolation, they require // translators: notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the fix: #57147

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block Theme Previews: Make it clearer what theme you're previewing
4 participants