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

Add Export (Clone) to site editor #292

Merged
merged 11 commits into from
Apr 3, 2023
Merged

Add Export (Clone) to site editor #292

merged 11 commits into from
Apr 3, 2023

Conversation

jffng
Copy link
Contributor

@jffng jffng commented Mar 27, 2023

This PR is a WIP 🔧 experiment to add CBT's export / clone functionality directly to the site editor.

Screenshot 2023-03-27 at 5 52 12 PM

src/editor.js Outdated Show resolved Hide resolved
@pbking
Copy link
Contributor

pbking commented Mar 28, 2023

I think the "save" functionality is more important than the "export" functionality. Perhaps we could add that as the first option?

If we "save" (rather than export) and then the native "export" functionality of Gutenberg is used, does that have the same result? (Perhaps we could just make the bits we add to the Editor a way to change the metadata and save (or "save as" if the slug is changed) simplifying what we're building and how it works with a user's workflow.

@jffng
Copy link
Contributor Author

jffng commented Mar 28, 2023

I think the "save" functionality is more important than the "export" functionality. Perhaps we could add that as the first option?

This isn't quite the feedback I've heard from designers, but I hear you that "save" makes sense to implement here.

If we "save" (rather than export) and then the native "export" functionality of Gutenberg is used, does that have the same result?

Not quite, there is functionality within CBT that does not exist within Gutenberg's export — particularly the escaping of relative URLs and patternizing of templates that contain images.

@jffng jffng force-pushed the try/site-editor-overwrite branch from da97b68 to 4870efb Compare March 28, 2023 17:53
src/editor.js Outdated Show resolved Hide resolved
src/editor.js Outdated
@@ -0,0 +1,154 @@
import { registerPlugin } from '@wordpress/plugins';
import { PluginSidebar, PluginSidebarMoreMenuItem } from '@wordpress/edit-site';
import { blockDefault } from '@wordpress/icons';
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we should use this icon

Copy link
Contributor Author

@jffng jffng Mar 29, 2023

Choose a reason for hiding this comment

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

I updated to use the tool icon but I'm not sure about this either, it's also rather generic.

src/editor.js Outdated Show resolved Hide resolved
src/editor.js Outdated Show resolved Hide resolved
src/editor.js Outdated Show resolved Hide resolved
src/editor.js Outdated Show resolved Hide resolved
src/editor.js Outdated Show resolved Hide resolved
src/editor.js Outdated Show resolved Hide resolved
@matiasbenedetto
Copy link
Contributor

I left a few comments about the code. In terms of UI it looks good.

I'm just wondering about:

  1. Is there consensus around moving all the interfaces out from wp-admin and into the editor even the plugin ones?
  2. If the answer to 1 is Yes: Will we add just one of the options of the plugin to the editor or all of them (export, clone, child, variation, etc)?

@jffng
Copy link
Contributor Author

jffng commented Mar 29, 2023

Thanks for the reviews!

Is there consensus around moving all the interfaces out from wp-admin and into the editor even the plugin ones?

I think moving all of the interfaces to the site editor is good goal to shoot for. I think the next phase of Gutenberg also focuses on extensibility, so iterating on CBT in the site editor will help inform what needs to be developed in Gutenberg. What is your opinion?

If the answer to 1 is Yes: Will we add just one of the options of the plugin to the editor or all of them (export, clone, child, variation, etc)?

Yes, but I think it makes sense to prioritize Export, Overwrite, and Style Variations.

Co-authored-by: Ben Dwyer <ben@scruffian.com>
@jffng jffng marked this pull request as ready for review March 29, 2023 21:29
@scruffian
Copy link
Contributor

Is there consensus around moving all the interfaces out from wp-admin and into the editor even the plugin ones?
If the answer to 1 is Yes: Will we add just one of the options of the plugin to the editor or all of them (export, clone, child, variation, etc)?

I believe all the UI should be in the site editor. That was always the intention but I just stuck it in wp-admin as a quick first pass. If I'd known how popular the plugin would become I'd have never put it there!

@scruffian
Copy link
Contributor

I think you can merge this when the linter is happy.

@pbking
Copy link
Contributor

pbking commented Mar 30, 2023

If the answer to 1 is Yes: Will we add just one of the options of the plugin to the editor or all of them (export, clone, child, variation, etc)?

Yes, but I think it makes sense to prioritize Export, Overwrite, and Style Variations.

I definitely agree with all of the above that we should be moving all aspects to the site editor. There are two things to consider doing this:

  • Any action other than "export" changes the Theme and the editor will need to re-load those changes. I've experimented with doing this for other tools (Revisr) but haven't found a good solution yet. It's not a matter when the actions are in wp-admin, but moving the actions into the Site Editor means that for those actions we'll need to plan to handle the changes.

  • It may be wise to revisit the actions and consider alternate paths; potentially consolidating or refactoring that UI rather than just moving the "here are all of the possible options" interface that's currently in wp-admin. For example we may want to introduce "New", "Save" and "Save As..." options that may better fit in people heads and simplify the actions. This could separate the "Metadata" editor from "export" since there are multiple use-cases for editing the metadata.

Moving the functionality into the Site Editor is the PERFECT opportunity to refactor that UI and I think that we should do that carefully. I'm not convinced that we should merge this in until we've had an opportunity to create a target for what that UI could be.

@jffng
Copy link
Contributor Author

jffng commented Mar 30, 2023

@pbking great feedback, thanks for raising those issues.

Any action other than "export" changes the Theme and the editor will need to re-load those changes

Generating a style variation was demo'd in #96, I'm not sure if it still requires a GB change but that is worth revisiting.

Overwrite does seem trickier.

It may be wise to revisit the actions and consider alternate paths; potentially consolidating or refactoring that UI rather than just moving the "here are all of the possible options" interface that's currently in wp-admin

Yep, I agree let's not just move all the options into the sidebar as is, but take the time to consider how they could be refactored or improved to fit better into the site editor model.

I'm not convinced that we should merge this in until we've had an opportunity to create a target for what that UI could be.

Do you have suggestions for how the Export functionality and the UI presented in this PR could be reworked? Or are you just suggesting to come up with a more holistic rework before merging anything and iterating?

@scruffian
Copy link
Contributor

Personally I don't see a need to put this PR on hold for the sake of future UX improvements. We don't need to maintain backwards compatibility, so we can just merge this and then improve it in another PR. Done is better than perfect.

@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Mar 31, 2023

I added a commit to set the filename of the zip exported. Without it, the user receives a zip file with a random name (example: 736a3448-d2c4-4b05-9e4b-13f42f1658d2.zip). If the user uploads the zip file with that filename, it becomes a problem because that random name is used as the theme slug name, and some parts of the site are broken.

Apart from that, it looks good to me. I think we can merge and continue iterating over the UI later.

@jffng
Copy link
Contributor Author

jffng commented Apr 3, 2023

I just thought of a small bug with this PR related to the theme's metadata — adding / editing tags is not supported yet in this flow, so I think we should carry through the tags of the current theme.

Fixed in fd779d3

@jffng jffng merged commit 3a58a30 into trunk Apr 3, 2023
@jffng jffng deleted the try/site-editor-overwrite branch April 3, 2023 15:27
mikachan added a commit that referenced this pull request Apr 5, 2023
commit 0374c69
Merge: 83de623 072da09
Author: Sarah Norris <sarah@sekai.co.uk>
Date:   Tue Apr 4 16:15:50 2023 +0100

    Merge pull request #297 from WordPress/add/font-family-labels

    Add labels around Google font family checkbox controls

commit 83de623
Merge: 58491a7 b814f29
Author: Sarah Norris <sarah@sekai.co.uk>
Date:   Tue Apr 4 16:13:48 2023 +0100

    Merge pull request #296 from WordPress/try/add-current-wp-version

    Add current WordPress version to style.css and readme.txt

commit 58491a7
Author: github-actions <github-actions@github.com>
Date:   Mon Apr 3 20:54:08 2023 +0000

    Version bump & changelog update

commit 3a58a30
Merge: bb36a3a fd779d3
Author: Jeff Ong <jonger4@gmail.com>
Date:   Mon Apr 3 11:27:38 2023 -0400

    Merge pull request #292 from WordPress/try/site-editor-overwrite

    Add Export (Clone) to site editor

commit fd779d3
Author: Jeff Ong <jonger4@gmail.com>
Date:   Mon Apr 3 11:26:11 2023 -0400

    Carry forward previous theme tags.

commit 072da09
Author: Sarah Norris <sarah@sekai.co.uk>
Date:   Sat Apr 1 21:39:54 2023 +0100

    Add checkbox labels to font family table header

commit 54318c6
Author: Sarah Norris <sarah@sekai.co.uk>
Date:   Sat Apr 1 21:39:22 2023 +0100

    Add checkbox labels to font family variants

commit b814f29
Author: Sarah Norris <sarah@sekai.co.uk>
Date:   Sat Apr 1 20:54:04 2023 +0100

    Add current WP version to readme.txt

commit f4070b2
Author: Sarah Norris <sarah@sekai.co.uk>
Date:   Sat Apr 1 20:53:50 2023 +0100

    Add current WP version to style.css

commit bb36a3a
Merge: b5a0a0a 8c4098a
Author: Matias Benedetto <matias.benedetto@gmail.com>
Date:   Fri Mar 31 17:35:57 2023 -0300

    Merge pull request #293 from WordPress/update/google-fonts-json-_4537839996

    Update Google Fonts JSON data from API

commit cd5f862
Author: Matias Benedetto <matias.benedetto@gmail.com>
Date:   Fri Mar 31 17:22:36 2023 -0300

    setting the filename of the exported zip file

commit b5a0a0a
Merge: e3f9193 260d115
Author: Matias Benedetto <matias.benedetto@gmail.com>
Date:   Fri Mar 31 13:05:52 2023 -0300

    Merge pull request #291 from WordPress/fix/280

    Fixing image downloading not working in some cases

commit 1b19d29
Author: Jeff Ong <jonger4@gmail.com>
Date:   Thu Mar 30 12:24:35 2023 -0400

    Make linter happy.

commit b86a1f1
Author: Jeff Ong <jonger4@gmail.com>
Date:   Thu Mar 30 12:23:42 2023 -0400

    Rename file.

commit 6efb440
Author: Jeff Ong <jonger4@gmail.com>
Date:   Wed Mar 29 17:27:42 2023 -0400

    Add validation to form button, improve spacing.

commit b5c6a17
Author: Jeff Ong <jonger4@gmail.com>
Date:   Wed Mar 29 17:11:01 2023 -0400

    Async/await, update icon, button.

commit 8bc38ef
Author: Jeff Ong <jonger4@gmail.com>
Date:   Wed Mar 29 16:40:00 2023 -0400

    Use tt3 as placeholder

    Co-authored-by: Ben Dwyer <ben@scruffian.com>

commit 16f3c5c
Author: Matias Benedetto <matias.benedetto@gmail.com>
Date:   Wed Mar 29 15:50:17 2023 -0300

    Implementing WordPress apiFetch replacing the browser's fetch function

commit 4870efb
Author: Jeff Ong <jonger4@gmail.com>
Date:   Mon Mar 27 17:47:44 2023 -0400

    Lint php.

commit ebdf773
Author: Jeff Ong <jonger4@gmail.com>
Date:   Mon Mar 27 17:47:22 2023 -0400

    Site editor rest API export.

commit a7b2de4
Author: Jeff Ong <jonger4@gmail.com>
Date:   Mon Mar 27 13:49:35 2023 -0400

    Add panel to sidebar in site editor.

commit 8c4098a
Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Date:   Tue Mar 28 00:15:24 2023 +0000

    Updating file

commit 260d115
Author: Matias Benedetto <matias.benedetto@gmail.com>
Date:   Mon Mar 27 15:48:44 2023 -0300

    fixing image downloading not working in some cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants