Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

@gigitux Thanks for working on this, I'm seeing a few issues:

1. In the first template sidebar preview there are some errors.
2. When I change to the second template and come back to the first, some headers and texts have disappeared.
3. When entering the assembler the first time, there's no toolbar to move the patterns, it only appears when clicking on another template and coming back to the first.

@gigitux Thanks for working on this, I'm seeing a few issues:

1. In the first template sidebar preview there are some errors.
2. When I change to the second template and come back to the first, some headers and texts have disappeared.
3. When entering the assembler the first time, there's no toolbar to move the patterns, it only appears when clicking on another template and coming back to the first.

CleanShot.2024-05-13.at.10.47.30.mp4

Thanks, @albarin, for the review!

1 & 2 are not specifically related to this PR. It looks like there is a regression on Gutenberg (you have the same issue if you try Gutenberg's trunk). We have to double-check!

When entering the assembler the first time, there's no toolbar to move the patterns, it only appears when clicking on another template and coming back to the first.

The UX about the toolbar will be handled in another issue: #47371

Hi @nefeline, @kmanijak,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

It looks like there is a regression on Gutenberg (you have the same issue if you try Gutenberg's trunk). We have to double-check!

Could we investigate if this issue is indeed a consequence of a regression on GB and not from this PR?

Could we investigate if this issue is indeed a consequence of a regression on GB and not from this PR?

I checked the WooCommerce trunk branch, and I'm able to reproduce the issue :)

Thanks for the updates @gigitux ! Sharing here my test results:

• Whenever accessing the editor with the GB plugin disabled, I'm greeted with an error:

Could we make a change to prevent this?

I checked the WooCommerce trunk branch, and I'm able to reproduce the issue :)

Cool, thanks: do you think its worth creating issues for these two? Maybe you already did, but flagging anyways just to make sure we are on the same page:

• In the first template sidebar preview there are some errors.
• When I change to the second template and come back to the first, some headers and texts have disappeared.

In the first template sidebar preview there are some errors.

This one is tracked by #47469!

When I change to the second template and come back to the first, some headers and texts have disappeared.

I'm not able to replicate it:
https://github.com/woocommerce/woocommerce/assets/4463174/2b247dc3-887e-47c2-a3c5-3b4f083f161b

Whenever accessing the editor with the GB plugin disabled, I'm greeted with an error:

I created an issue: #47545. I'm going to prioritize this!

Thanks!

I'm not able to replicate it:

Hm, yeah, I can't replicate it either. But I can replicate on this PR the same issue observed on #47356 regarding the scroll:

Just to confirm: do we have an issue open to solve this problem, in particular? If we do and we can prioritize it, I'm a-ok with approving/merging this PR 👌

Ah: can you also please update the description of this PR so it accounts for the changes on the feature flag & the fact that the PR on GB has already been merged? Thanks!

Just to confirm: do we have an issue open to solve this problem, in particular? If we do and we can prioritize it, I'm a-ok with approving/merging this PR 👌

I created a dedicated issue: #47570 - are you able to reproduce this issue on #47415?

Ah: can you also please update the description of this PR so it accounts for the changes on the feature flag & the fact that the PR on GB has already been merged? Thanks!

Done! Thanks for the review!

Done! Thanks for the review!

Thanks @gigitux ! Do you mind double-checking the testing instructions to see if they are accurate? I'm still seeing references to IS_TOOLBAR_ENABLED_FEATURE_FLAG.

Thanks for updating the PR testing instructions again 🙏

The notes I have here are very similar to the ones I shared on #47356 (review)

1- I think this was mentioned here earlier, but whenever clicking on "Design your homepage" for the very first time, the toolbar to move patterns is not displayed on the first template.
2. Even after the user leaves the "Design your homepage" section, they can still see the tooltip to move the patterns.

I'm ok with addressing those separately, as long as we have issues to solve the aforementioned problems.

Regarding the scroll problem:

are you able to reproduce this issue on #47415?

No, I'm not able to reproduce on #47415 : does this mean that you solved the problem via 47415? If yes, it would be nice to document those things here on this PR so the reviewers are aware of known issues and how they are being solved.

No, I'm not able to reproduce on #47415 : does this mean that you solved the problem via 47415? If yes, it would be nice to document those things here on this PR so the reviewers are aware of known issues and how they are being solved.

I changed nothing in the specific, so I'm not sure. It is better to keep the issue open.

1- I think this was mentioned here earlier, but whenever clicking on "Design your homepage" for the very first time, the toolbar to move patterns is not displayed on the first template.

#47752

2. Even after the user leaves the "Design your homepage" section, they can still see the tooltip to move the patterns.

#47751

Created!

I changed nothing in the specific, so I'm not sure. It is better to keep the issue open.

Do you think you can determine whether this PR introduces this bug? I'm concerned about approving it, merging it as-is, and then not being able to scroll up and down in the frame anymore. This type of bug can block us from doing other unrelated work down the line.