Skip to content

Conversation

@roykho
Copy link
Contributor

@roykho roykho commented Mar 24, 2021

All Submissions:

Changes proposed in this Pull Request:

So I was not able to reproduce the original stated issue similar to Vedansu's findings. I was however able to reproduce some of the other issues mentioned on the thread and this PR serves to correct those issues.

Twenty Twenty One theme.
Before:

After:

Before:

After:

Closes #28700

How to test the changes in this Pull Request:

NOTE: Don't forget to build the assets to test this.

  1. Activate Twenty Twenty One theme.
  2. Add couple products to the cart and go to the cart
  3. Ensure the table header rows are left aligned instead of center aligned. Refer to the images above.
  4. Go through with the checkout.
  5. Go to My Account and review the order you have just placed.
  6. Ensure the table header rows are left aligned instead of center aligned. Refer to the images above.

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?

Changelog entry

Tweak - Adjust Twenty Twenty One order items header alignment.

@roykho roykho requested review from a team and barryhughes and removed request for a team March 24, 2021 20:46
@barryhughes
Copy link
Member

💥 Can confirm that the problems with text being inappropriately centered in some of the table headers (within the cart and order confirmation screens when Twenty Twenty-One is activated) is fixed. Nice work!

😕 With trunk checked out and the latest versions of Twenty Twenty / One checked out I also was unable to see exactly the same issues documented in #28700, but I did observe a couple of possibly related things. Noting them below.


Vertical alignment (padding top: 1px)

2020 checkout-page dl-dt-p-1px-top-padding

It's actually quite hard to see, but with reference to the above screenshot—and this can be seen with Twent Twenty and Twenty Twenty-One—the selected variation text (dd p → "Consequatur odit") is 1px lower than the variation name (dt → "Quae"). This is coming from the following spots:

I'm wondering if we used to need these padding adjustments, but no longer do with the latest versions of these themes?

Horizontal alignment

The other thing that caught my eye (not actually sure this counts as a bug) is that, if you look at the previous screenshot from the checkout page, the selected variations are horizontally aligned below the product name. On the order confirmation page, though, they are indented:

2020 confirmation-page indented-vars

That's coming from the theme, and it's because we use an unordered list here (on the checkout page, it's a definition list and is styled differently). Again, not sure if we actually care about this one (?).

@barryhughes
Copy link
Member

☝️ @roykho the specific fix you've already committed is 💯 but what are your thoughts on those two further items? Worth addressing either (and as part of this PR or separately), or nay?

@roykho
Copy link
Contributor Author

roykho commented Mar 24, 2021

Hey @barryhughes - so I only addressed what was described in the issue. Anything outside of that should be its own issue I would say.

@barryhughes
Copy link
Member

barryhughes commented Mar 24, 2021

@roykho—for sure...just for clarity though I'll quickly note I was thinking of those two items as being related to this artifact (shared by Global Step in the original issue):

Screenshot_20210324_155531

I'm perhaps misinterpreting, and certainly this change is A-OK, so will go ahead and merge 👍

@barryhughes barryhughes merged commit 49e9630 into trunk Mar 24, 2021
@barryhughes barryhughes deleted the fix/28700 branch March 24, 2021 23:00
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Mar 24, 2021
@jonathansadowski jonathansadowski added this to the 5.3.0 milestone Apr 16, 2021
@tammullen tammullen added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: add changelog Mark all PRs that have not had their changelog entries added. [auto]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[GlobalStep - WooCommerce 4.9 RC1] Variable product misalignment on My Account + order subtotals misalignment during checkout

6 participants