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

Replace/admin interface #637

Merged
merged 53 commits into from
May 28, 2024
Merged

Replace/admin interface #637

merged 53 commits into from
May 28, 2024

Conversation

pbking
Copy link
Contributor

@pbking pbking commented May 14, 2024

Replace the wp-admin interface with a React app that does the following:

  • Export the theme zip
  • Creates a new theme (Blank, clone child)
  • Provides helpful information including details about the plugin, links to external resources and FAQs
image image

@pbking pbking marked this pull request as draft May 14, 2024 15:28
@beafialho
Copy link

I did a quick mockup of what this interface could look like, with a clearer content hierarchy and CTA's. The modal you shared above looks good to me when clicking each option.

CBT Interface

@pbking pbking self-assigned this May 21, 2024
@pbking pbking marked this pull request as ready for review May 21, 2024 15:55
Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

Took the liberty to:

  1. Add missing translations
  2. Change FAQ images to webp, which gives us a smaller filesize

includes/class-create-block-theme-editor-tools.php Outdated Show resolved Hide resolved
includes/class-create-block-theme-admin-landing.php Outdated Show resolved Hide resolved
pbking and others added 3 commits May 22, 2024 07:22
Co-authored-by: Vicente Canales <1157901+vcanales@users.noreply.github.com>
Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few nitpicks regarding eslint rule deactivation

src/landing-page/create-modal.js Outdated Show resolved Hide resolved
src/landing-page/create-modal.js Outdated Show resolved Hide resolved
src/landing-page/landing-page.js Outdated Show resolved Hide resolved
@pbking
Copy link
Contributor Author

pbking commented May 22, 2024

I believe all of the feedback has been addressed with one exception (that I'm not sure I understand).

There was some great reviewing in here!

Maybe ready for another look?

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

h1 no longer exists in the content 😅 I suggest making the title logo an h1, what do you think? In that case, I think we need to change the image alt from Create Block Theme Logo to Create Block Theme.

image

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the update! I have made additional suggestions.

Another thing that bothers me is that the translation files don't seem to be loading.

This plugin is already translated into several locales. And if I switch the site language to that locale, the script with the handle create-block-theme-app-js-translations should be enqueued, but it isn't.

For example, it is 100% translated in the Japanese locale, and the editor sidebar is fully translated. I was hoping that this new admin page would translate at least some of it, but it doesn't translate at all.

Will this not be translated unless the plugin is released and reflected in GlotPress?

src/landing-page/create-modal.js Outdated Show resolved Hide resolved
includes/class-create-block-theme-admin-landing.php Outdated Show resolved Hide resolved
includes/class-create-block-theme-admin-landing.php Outdated Show resolved Hide resolved
src/landing-page/landing-page.js Outdated Show resolved Hide resolved
src/admin-landing-page.scss Show resolved Hide resolved
pbking and others added 2 commits May 23, 2024 12:32
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
@pbking
Copy link
Contributor Author

pbking commented May 24, 2024

Latest round of adjustments are in. Ready for another review I think!

@pbking
Copy link
Contributor Author

pbking commented May 24, 2024

Another thing that bothers me is that the translation files don't seem to be loading.
...
Will this not be translated unless the plugin is released and reflected in GlotPress?

I've no idea why the translations wouldn't be working as expected. That might be related... Should we release this change and see?

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

👍

@t-hamano t-hamano self-requested a review May 24, 2024 15:36
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

I think if we address the comment left by @vcanales, we can ship this PR.

Another thing that bothers me is that the translation files don't seem to be loading.

I checked the return value of wp_set_script_translations() and it is true, so there should be no problem with the implementation. Let's see if the translations will be reflected when this version is released and localized.

pbking and others added 2 commits May 28, 2024 07:44
Co-authored-by: Vicente Canales <1157901+vcanales@users.noreply.github.com>
@pbking pbking merged commit 2b17395 into trunk May 28, 2024
2 checks passed
@pbking pbking deleted the replace/admin-interface branch May 28, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants