-
Notifications
You must be signed in to change notification settings - Fork 219
Refactor Cart and Checkout Page Templates #10773
Conversation
Confirm version to run migration.Confirm version to run migration.
woocommerce-blocks/src/Migration.php Lines 24 to 31 in 970c136
🚀 This comment was generated by the automations bot based on a
|
The release ZIP for this PR is accessible via:
Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/blocks/classic-shortcode/index.tsx
assets/js/blocks/page-content-wrapper/index.tsx assets/js/editor-components/default-notice/index.tsx |
Size Change: +9.87 kB (+1%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
Thanks Mike, I'll spin up the PR and take a look sometime today. |
I haven't looked at any code and just focused on a couple flows here. Generally in my testing so far this is looking pretty good but there are still some user gotchas that have surfaced so far. With classic themes things seem to be working as expected, however, some unexpected things happening (or user gotchas) when using a block theme: What I noted in the video: CleanShot.2023-08-30.at.16.17.54.mp4
Not in the video:
There are a bunch of things in here so before I jump into suggested changes I'm interested in getting your thoughts on the above observations (in case some aren't reproducible or misunderstandings) before I jump into suggested solutions. |
Thanks for the detailed review @nerrad. I agree with those criticisms, and some apply to the current approach also. it certainly is disjointed how we require both a page and a template with both approaches. Certainly something we need to solve.
We've been noticing issues like this for a while; it seems direct navigation to the site editor prevents certain functionality from loading correctly until you back out and go back in. This appears to be a Gutenberg issue which we'll need to raise upstream with steps to reproduce. On our side I'm not sure what we can do aside from avoid direct links to templates.
Agreed. I think if we pursue this PR further we should name it something like "Cart Page Placeholder" just so its clear what it is going to render unless you replace it.
Something I've noticed with other page templates is that when the template uses post content/title blocks, the content can be edited in the site editor without going to the template itself: This improves the page editing experience, but it makes the template editing experience slightly worse because you only see generic placeholders in the template itself: This comes down to the disconnect between pages and templates. cc @wavvves @ralucaStan It kind of feels like we should have one or the other, not both. I know one of the main motivations for templates was originally the distraction free views for checkout. Were there more reasons? I think the pages section in site editor is quite new; I wonder if this was maybe added after speccing the project? Does the existence of that lessen the need for dedicated templates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @mikejolley. I've followed the testing instructions and mostly everything works ok. I did run into a few things that may or may not be caused by this PR, but seemed relevant, and left some general thoughts as I was going through it.
- I agree with Darren that there is a disconnect between pages and templates and it feels very confusing going between them. Don't really have a solution here but I was trying to go through it wearing the hat of a typical user.
- Saying that, I get that the pages are necessary, otherwise the user would loose the cart and checkout if they switch to a non-block template
- I also ran into the issue at the end of Darren's video where the legacy template block is not recognised.
- I also ran into the following issue, which I took a leaf out of Darren's book and recorded it
Finding.a.Solution.for.the.Cart.Page.Issue.mp4
I didn't look at the code yet, but I will once we're committed to this solution.
cd78612
to
0f77247
Compare
@nerrad @ralucaStan @wavvves I've pushed the revisions that we discussed.
Let me know if there are any questions. I added a todo item to remove/adjust the page migration code that exists already. |
Endpoint revert b359dc7 |
More notes will come but just wanted to comment quickly on this before I forget. This step does work, however what is the expected behaviour with this branch merged as is for the following scenarios (including considerations for folks that may have an earlier version of the templates work):
As a note, my initial test of this branch was (not following the instructions about deleting pages):
When I visited the template and the pages, I was just presented with the shortcode block for the cart and checkout. Hence, my questions above regarding expectations for this branch behaviour as is. I want to make sure that we are aligned on what the expectations should be when this branch is merged. |
I'm looking at the branch. I used my local sites that has WC Version 8.0.2 and had this value The Cart block worked as expected, but I wanted to show the errors I see with The Checkout block: https://www.loom.com/share/0dfa26452a5e4b6eaf7c2d41b2268e09?sid=c095d570-5e49-418a-bb89-730efef34554 |
We had a demo of the branch with @nerrad, @elizaan36 and @pmcpinto. We decided some minor changes are needed on this branch; @pmcpinto and @elizaan36 can help with the text changes
@nerrad said he will add some missing testing scenarios There was an open question if the placeholder is showing on top of the shortcode or on top of the whole page content and what should the experience look like |
I see. In that case, please disregard my prior comment. |
OK. Got it. 👍 |
@nielslange You went to the pages editor instead of the templates editor? How can I clarify this; It does say "templates". |
After reverting prior changes and executing the testing steps again, the problem no longer appears, and I can see the shortcode cart and checkout, when using a classic theme, and the Cart and Checkout blocks, when using a block theme. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first test case states the following:
- Go to Appearance > Editor > Templates and edit the Page: Checkout template. Edit something in the template, for example, something under the content. Save and confirm the frontend shows your change.
- Go to Appearance > Editor > Templates and edit the Page: Checkout template. Click the three dots in the side bar and "revert customisations". Ensure the frontend shows the same default template again without your changes.
I went ahead and added the following paragraph inside the Shipping Method
inner block:
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
As expected, this paragraph appeared both in the page editor and on the frontend. I then cleared the customisations, but the paragraph remained. The following screenshot shows, that there are no customisations, that can be reverted. Yet, the paragraph remains visible. Is that expected, as I've added a block rather than adjusting a setting within the Checkout block? 🤔
@nielslange You've edited the page rather than the template. When you saved it would have also said its changing the page content. You can see what you're within using the breadcrumbs at the base of the editor. |
I've added a dev note to the PR description. |
The problem I'm facing is, that I'm unable to revert the change, unless I go into either the template or the page and manually remove the added paragraph. In the site editor, when viewing "Pages:, I can only trash the page: In the site editor, when viewing "Templates", I see "Clear customizations": Clicking on "Clear customizations" shows me a notice that the site had been reverted, but my added paragraph remains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@opr This depends on where the edit was made. If the edit was made to the template, no it won't be visible in the page editor, but it will be visible on the frontend. The page content can also be edited via the template, so check the path at the bottom of the editor to see if you're within the page placeholder block.
Can you confirm?
That's it, it's working fine now!
One more comment about the Classic Shortcode Block, I think the UX would be better if the "modal" always showed without needing to hover/select the block.
Yeah I did think about this, but we're following the pattern of the classic template block which is on select also. @opr change both or leave it? |
@mikejolley Have we already had guidance from design about this and they're fine with the classic template block's behaviour? If so, we can leave it, but if not then I think we should change it 🙏🏼 |
@opr we have not but classic template exists already so I assume it must have? I'm happy to leave it and tackle in a follow up if not. Ideally all would be done at the same time since they share this pattern. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@opr we have not but classic template exists already so I assume it must have? I'm happy to leave it and tackle in a follow up if not. Ideally all would be done at the same time since they share this pattern.
Yeah OK let's do it in a follow up, it would expand the scope of this PR so better to deal with it separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, and addressing my previous feedback. The PR works as expected now, @mikejolley. 🙌
Introduces a Page Content Wrapper and Classic Shortcode block to improve the merchant facing experience when dealing with templates, pages, and the legacy shortcodes.
What
Supersedes #10666
EDIT:
Based on the feedback and the disconnect between pages and templates, this now:
TODO:
Why
Change Checkout Template Slug from
checkout
topage-checkout
andcart
topage-cart
This seemingly small change means that the template would be woocommerce//page-checkout and page-checkout in the template hierarchy.
What this fixes
Theme compatibility. Any theme that has a page-checkout.html block template will be respected again, and that template will take priority over our version (which is what we want).
No migration – use new block placeholder instead
The migration routine takes the content from the original checkout page and inserts it into a custom checkout template. This however has caused issues with failed migrations and block templates which are difficult to detect.
The solution I propose is to instead make the default template contain a placeholder which renders the page content directly from the page instead of copying it. This allows us to maintain compatibility with pages, whilst also allowing the merchant to customise the template or switch to blocks in the site editor easily.
The default template gets a new legacy/classic block placeholder which looks like this:The default template includes a placeholder which loads page content via Inner Blocks. This allows for inline editing of page content from one location.
It also offers a button to convert to a blockified checkout instead. The same is implemented for the cart page.
What this fixes
Deprecation of the previous checkout template
Because some users may have already upgraded and customised the
checkout
template, there is a small migration routine on upgrade which renameswoocommerce//checkout
towoocommerce//page-checkout
. This is a one off upgrade process which prevents duplicates in the site editor, and moves any customisations to the new template. There is a todo to update the versioning this runs on depending on the release this PR ships with.Other feedback
This PR has had some suggestions.
Addressing pages as a top-level item in the site editor
Latest Gutenberg has a new page editor which shows some special dynamic pages (404, search) below the rest of the pages. This is currently not extendable, and may not be a good fit because there is still a Checkout/Cart page, so I have not implemented this.
Surfacing reversion
Once you've converted to blocks, you can only revert by reverting the template. There may be ways to make this more obvious, but I'm not sure its worth it vs documenting it somewhere.You cannot revert, but since edits are kept within pages you can revert to an earlier revision or put the block/shortcode back yourself.
Testing Instructions
Before testing, take note of the frontend appearance of the cart/checkout pages.
Page: Cart
andPage: Checkout
. For both, click the three dots in the inspector and "revert customisations". This will remove any existing template customisations or past migration.Classic Shortcode Block
Page Permalink Management 2185e59
Test Page Wrapper on Site Editor a34c74d
Developer testing
WooCommerce Visibility
Required:
Checklist
Required:
[type]
label or a[skip-changelog]
label.Conditional:
[skip-changelog]
label is not present).Changelog
Dev Note
In Blocks 10.6.0 new templates were introduced to control the layout of Cart and Checkout pages specifically for block theme. The goal was to provide a single editing experience for cart/checkout pages and move away from the page editor, and to introduce more condensed versions of headers/footers for those pages to prevent distractions during checkout. This change however introduced some conflicts with themes implementing their own page templates, and well as making it more difficult to find and edit page content for those familiar with the traditional WooCommerce editing experience.
To improve this, we've rolled out several improvements which not only aid discoverability of templates and compatibility with themes, but also prevent the need for complex migrations between pages and templates. To summarise the changes:
page-cart
andpage-checkout
for greater compatibility with the page template hierarchy and themes. Themes that implement apage-cart.html
or similar will take priority over the default templates in WooCommerce. Custom templates created by the merchant take ultimate priority over both.Admin > Pages
orAppearance > Editor > Pages
has been restored. This content is displayed within the new default cart and checkout templates.Appearance > Editor > Templates > Page: Cart / Page: Checkout
for convenience. On save, both entities will be saved.For users of block themes who have already upgraded to WC 8.0 and already had their templates migrated, those templates will be renamed on upgrade, but the content will not be updated. That means you'll likely have a template that has the previous page content hardcoded within it. This is completely optional, but if you'd like to change this back to pulling content from the real page, do the following:
Page: Cart
andPage: Checkout
you'll see an icon to the write which brings up some options.Any content you had in these pages previously (if you already upgraded to the last version of WooCommerce) will be restored.