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

fixed misaligned elements in generated refunds page issue #45292

Conversation

SpaceSurfer1
Copy link
Contributor

@SpaceSurfer1 SpaceSurfer1 commented Mar 4, 2024

Fixed misaligned elements in generated refunds page issue.

Submission Review Guidelines:

Changes proposed in this Pull Request:

Closes #45213 .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Go to WP Admin > Pages. If your site already has a "Refund and Returns Policy" page, delete it.
  2. Using wp-cli, run wp shell. Then in the shell, run WC_Install::create_pages(). This will regenerate the Refund page with the updated content from this PR.
  3. Back on WP Admin > Pages, click the link to preview the "Refund and Returns Policy" page. The "Late or missing refunds" and "Sale items" headings should now be aligned correctly.

Changelog entry

Fixed misaligned elements in generated refunds page issue, updated tags.

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Fix alignment issues in the generated content of the Refunds page

Comment

Fixed misaligned elements in generated refunds page issue.
@github-actions github-actions bot added plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution labels Mar 4, 2024
@woocommercebot woocommercebot requested review from a team and coreymckrill and removed request for a team March 4, 2024 18:15
Copy link
Contributor

github-actions bot commented Mar 4, 2024

Hi ,

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

@coreymckrill
Copy link
Collaborator

@SpaceSurfer1 thanks for the PR! In my testing, looks like this does indeed fix the issue. I wonder if it would be even better, from an HTML semantics perspective, if those two lines were changed to h3 tags instead of paragraphs. What do you think?

@SpaceSurfer1
Copy link
Contributor Author

Hi @coreymckrill, Thanks for reviewing👍. I just tested with h3 tag and compared with present one, following is the screenshot -

Screenshot 2024-03-05 at 2 09 19 AM

The font and size is changed while using h3. <p><b> keeps it same as before. Any suggestions?

@coreymckrill
Copy link
Collaborator

🤔 I guess it depends on which theme you're using. It looks like you're testing on Twenty Twenty Four, which is what I'm using too. With the h3, it is definitely bigger than it was before, but it's still smaller than the h2 heading that it belongs to:

refunds

I'll check and see if anyone else has thoughts on this...

@SpaceSurfer1
Copy link
Contributor Author

@coreymckrill , Sure....👍

@coreymckrill
Copy link
Collaborator

@SpaceSurfer1 given that this change doesn't affect current stores, only new ones (since the content is only generated once when WooCommerce is first installed), I'd say let's go ahead and improve the semantics and switch to h3s.

This actually brings up a few other issues with the generated content that you could also address in this PR if you want to (also fine to just keep it focused on the two lines):

  • "Overview" is currently an h3, but it should probably be an h2 to match the other heading levels
  • All of the headings are wrapped in the wrong block "grammar". Meaning they are wrapped in <!-- wp:paragraph --> block tags, but they should have <!-- wp:heading --> instead.
  • The heading tags should have a class="wp-block-heading" attribute, since that's what you get when you insert a heading block into the editor manually.

So the "Overview" heading, for example, should look like this:

<!-- wp:heading -->
<h2 class="wp-block-heading">Overview</h2>
<!-- /wp:heading -->

If you'd rather not take on the extra changes, we can spin up another PR later.

@SpaceSurfer1
Copy link
Contributor Author

Hi @coreymckrill , I'll address those additional changes in this PR only. 👍

Updated tags to <h3>, updated block tags and added class attribute for headings
@SpaceSurfer1
Copy link
Contributor Author

Hi @coreymckrill , I have updated the tags and added class attribute👍 . Can you please review it? Thanks.

Copy link
Collaborator

@coreymckrill coreymckrill left a comment

Choose a reason for hiding this comment

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

👍 All looks good now

@coreymckrill coreymckrill merged commit 7ded649 into woocommerce:trunk Mar 15, 2024
31 of 33 checks passed
@github-actions github-actions bot added this to the 8.8.0 milestone Mar 15, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 15, 2024
@alopezari alopezari added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris type: community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misaligned elements in generated refund page
3 participants