Skip to content

Conversation

@dinhtungdu
Copy link
Member

@dinhtungdu dinhtungdu commented May 2, 2022

All Submissions:

Changes proposed in this Pull Request:

There are some issues with the current Twenty Twenty-Two integration (which is detailed in woocommerce/woocommerce-blocks#5863):

  • The wrapper with of Single Product and Product Catalog page is fixed to 1000px and can't be changed.
  • The current implementation breaks the alignment and alignment setting of the Classic Template block.

This PR fixes those issues by removing the hard-coded width and adding the wrapper back to fix the alignment issue.

The initial width of the wrapper will be set to 650px. This can be fixed in woocommerce/woocommerce-gutenberg-products-block by setting the default alignment of the Classic Template block to wide.
Update: The default layout was changed to wide in woocommerce/woocommerce-blocks#6356.

Fixes woocommerce/woocommerce-blocks#5863.

How to test the changes in this Pull Request:

  1. With Twenty Twenty-Two, edit the Product Catalog template.
  2. Select the Group block that contains the Classic Template.
  3. Set an arbitrary background color.
  4. On a new tab, go to the shop page on the front end, see the background is set and the layout doesn't break.
  5. Back to the Site Editor tab, select the Classic Template block, and set the alignment to Wide.
  6. See it reflects on the front end.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Have you created a changelog file by running pnpm nx affected --target=changelog?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 2, 2022
@dinhtungdu dinhtungdu marked this pull request as draft May 2, 2022 11:35
@dinhtungdu dinhtungdu marked this pull request as ready for review May 2, 2022 11:51
@Aljullu Aljullu requested a review from vedanshujain May 2, 2022 11:56
Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this @dinhtungdu!

I will leave somebody from Proton do the final review.

@dinhtungdu dinhtungdu changed the title fix: Twenty Twenty-Two integration Fix: Twenty Twenty-Two integration May 3, 2022
vedanshujain
vedanshujain previously approved these changes May 3, 2022
@vedanshujain
Copy link
Contributor

seems like tests are failing but seems like an intermittent error, i am restarting them to see if that fixes it.

@dinhtungdu
Copy link
Member Author

dinhtungdu commented May 3, 2022

@vedanshujain @Aljullu I've just realized that we lose the wrapper width of the checkout page.

In 043aaf4, I add the wrapper width back and remove the unnecessary style for the account page. Please give this another review!

Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

@vedanshujain @Aljullu I've just realized that we lose the wrapper width of the checkout page.

Good catch! New changes look good to me.

@Aljullu Aljullu requested a review from vedanshujain May 26, 2022 14:37
vedanshujain
vedanshujain previously approved these changes May 27, 2022
@Aljullu Aljullu requested a review from vedanshujain June 2, 2022 08:12
@Aljullu
Copy link
Contributor

Aljullu commented Jun 2, 2022

@vedanshujain can you merge this PR? It looks like neither @dinhtungdu neither I can merge it.

imatge

@vedanshujain vedanshujain merged commit 7de6d18 into woocommerce:trunk Jun 2, 2022
@github-actions github-actions bot added this to the 6.7.0 milestone Jun 2, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2022

Hi @vedanshujain, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

@vedanshujain vedanshujain added the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin. release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Twenty Twenty-Two: WooCommerce templates might not take the full width in certain circumstances

3 participants