Skip to content

Live Preview: Show the current theme name on the theme activation modal#57588

Merged
arthur791004 merged 4 commits intoWordPress:trunkfrom
arthur791004:feat/show-the-current-theme-on-the-theme-activation-modal
Jan 12, 2024
Merged

Live Preview: Show the current theme name on the theme activation modal#57588
arthur791004 merged 4 commits intoWordPress:trunkfrom
arthur791004:feat/show-the-current-theme-on-the-theme-activation-modal

Conversation

@arthur791004
Copy link
Copy Markdown
Contributor

@arthur791004 arthur791004 commented Jan 5, 2024

What?

Display the name of the currently active theme on the theme activation modal

Why?

Resolved Automattic/wp-calypso#81878

How?

Use the apiFetch with wp_theme_preview= to bypass the createThemePreviewMiddleware and get the currently active theme.

Another way is to create a new resolver, selector, and reducer to get the raw current theme and save it into the redux store. But I think it's only used by this modal now so we don't need to introduce new variable into the redux store.

Testing Instructions

  1. Go to /wp-admin/themes.php
  2. Preview a theme
  3. Click the “Activate” button
  4. Make sure the name of the currently active theme is shown in the description

Testing Instructions for Keyboard

Screenshots or screencast

image

@arthur791004 arthur791004 self-assigned this Jan 5, 2024
@arthur791004 arthur791004 removed request for mmtr and nerrad January 8, 2024 03:45
@arthur791004 arthur791004 force-pushed the feat/show-the-current-theme-on-the-theme-activation-modal branch from 22e7337 to 142d29d Compare January 8, 2024 03:48
} );

apiFetch( { path } ).then( ( activeThemes ) =>
setCurrentTheme( activeThemes[ 0 ] )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it make sense to handle a request error here? Wondering if it's safe to access activeThemes[ 0 ] here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Handled the request error by 2f87b34.

Wondering if it's safe to access activeThemes[ 0 ] here.

It should be safe as we also access the value here.

Copy link
Copy Markdown
Contributor

@miksansegundo miksansegundo left a comment

Choose a reason for hiding this comment

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

LGTM 👍 It works as expected.

Screenshot 2567-01-08 at 19 11 39

@arthur791004 arthur791004 added the [Type] Enhancement A suggestion for improvement. label Jan 9, 2024
Copy link
Copy Markdown
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.

Looks good to me 🎉

themeName
/* translators: %1$s: The name of active theme, %2$s: The name of theme to be activated. */
__(
'Saving your changes will change your active theme from %1$s to %2$s'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We're missing the last period (.) here; it was there before this PR.

@arthur791004 arthur791004 merged commit 44ba13d into WordPress:trunk Jan 12, 2024
@arthur791004 arthur791004 deleted the feat/show-the-current-theme-on-the-theme-activation-modal branch January 12, 2024 08:16
@github-actions github-actions bot added this to the Gutenberg 17.6 milestone Jan 12, 2024
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.

Live Preview: Shows the name of the current theme in the prompt text

4 participants