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

Add tag name configurability to global footer block #620

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

adamwoodnz
Copy link
Contributor

@adamwoodnz adamwoodnz commented Jun 5, 2024

See WordPress/Learn#2501 and https://wordpress.slack.com/archives/C01KC5VDQBC/p1717552327365119?thread_ts=1717455021.461299&cid=C01KC5VDQBC

On Learn there is a site footer above the global footer. This is currently between the main and the footer landmark provided by the global footer block. For accessibility reasons there shouldn't be content between these landmarks.

This PR adds the ability for the tagName to be configured so that the global footer can be nested within a footer element and maintain valid HTML.

Can be tested using WordPress/Learn#2502

There should be no effect in other themes as the default value for the tagName attribute is set to footer.

Before

Screenshot 2024-06-05 at 2 38 30 PM

After

Screenshot 2024-06-05 at 2 52 30 PM

@adamwoodnz adamwoodnz self-assigned this Jun 5, 2024
@adamwoodnz adamwoodnz added the Accessibility Issues related to keyboard navigation, screen readers, etc label Jun 5, 2024
@adamwoodnz adamwoodnz requested a review from ryelle June 5, 2024 02:59
@@ -847,8 +847,10 @@ function render_global_footer( $attributes, $content, $block ) {
$wrapper_attributes = get_block_wrapper_attributes(
array( 'class' => 'global-footer wp-block-group' )
);
$tag_name = $attributes['tagName'] ?? 'footer';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this should be validated somehow, maybe one of a list of possible tag names? I guess it would just be div or footer, in reality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a good idea imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 433b5c7

@adamwoodnz adamwoodnz merged commit 308c936 into trunk Jun 5, 2024
2 checks passed
@adamwoodnz adamwoodnz deleted the enhance/global-footer-tagname branch June 5, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Issues related to keyboard navigation, screen readers, etc [Block] Header & Footer
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants