Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Add <main> to "Offset post without featured image" #318

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Sep 14, 2024

Description
Adds the <main> HTML tag to the pattern. All templates must include a main, both because all content needs to be inside a landmark, but also because in block themes, the automated skip link requires it.

Indent the blocks in the file to improve the readability of the code.

Partial for #278

Screenshots

Testing Instructions

Apply the PR and go to Appearance > Editor > Templates.
Select the single post template.
In the settings sidebar, open the Template tab.
Open the Design panel and select the "Offset post without featured image" design.
Open the list view.
Select the group block that is between the header and footer.
In the block settings sidebar, open the Advanced panel.
Confirm that main is selected in the HTML element option.
Save.
View a single post on the front.
Confirm that there are no issues with the layout, the template should look identical before and after adding the main.

Indent the blocks in the file to improve the readability of the code.
@carolinan carolinan added Accessibility (a11y) Needs accessibility testing or feedback [Component] Block Patterns labels Sep 14, 2024
@carolinan carolinan marked this pull request as ready for review September 14, 2024 13:28
Copy link

github-actions bot commented Sep 14, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: carolinan <[email protected]>
Co-authored-by: juanfra <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@carolinan carolinan added the [Type] Bug An existing feature does not function as intended. label Sep 14, 2024
Copy link
Member

@juanfra juanfra left a comment

Choose a reason for hiding this comment

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

Looks good - I found a couple of is-style-default appearances that we may want to remove now.

@carolinan
Copy link
Contributor Author

It looks like there were many changes to the file, but that is mostly because of the indentation.

I removed is-style-default.
I renamed it to template-single-offset based on the discussion here: #244
And I replaced the comments area markup with the comments pattern, to avoid repeating code.
I checked that the previous comment area was identical to the pattern.

To do:

  • Internationalization - add translation functions
  • Check if the Post navigation pattern can be used, to avoid repeating code.

@carolinan carolinan requested a review from juanfra September 17, 2024 12:55
Copy link
Member

@juanfra juanfra left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks Carolina!

For i18n, I've added this PR that will probably help us with that.

Regarding reusing the post pagination pattern, I believe we could tackle that in a separate template, and do it in all the templates where it's needed.

@carolinan
Copy link
Contributor Author

I found that the spacing is different between the post navigation in this pattern and in the the post navigation pattern.
It can be sorted, but yeah lets get these fixes in first.

@carolinan carolinan merged commit 5211a80 into trunk Sep 17, 2024
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility (a11y) Needs accessibility testing or feedback [Component] Block Patterns [Type] Bug An existing feature does not function as intended.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants