#47665 closed defect (bug) (wontfix)
validator.w3.org errors
Reported by: | Omer Faruk Ekinci | Owned by: | audrasjb |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Gallery | Keywords: | has-patch needs-testing has-unit-tests |
Focuses: | Cc: |
Description
Attachments (1)
Change History (12)
This ticket was mentioned in Slack in #core by hareesh-pillai. View the logs.
5 years ago
#7
@
5 years ago
- Keywords 2nd-opinion removed
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from reviewing to closed
Thank you for the bug report, @omer-faruk-ekinci!
While this report is valid, I'm going to close this ticket, for a few reasons:
- This seems to only be an issue for the w3 validator, there are no reports of browsers or screen readers having issues with the lack of
<dd>
element. - This only affects the gallery shortcode in the classic editor (or classic block in the block editor). Both of these use cases are heading in to long term maintenance mode, during which minor bug fixes are unlikely to land.
- It would require a non-trivial amount of time to test and ensure this change doesn't accidentally cause other issues.
This ticket was mentioned in PR #3973 on WordPress/wordpress-develop by @andrewserong.
20 months ago
#9
- Keywords has-unit-tests added
This PR backports the PHP parts of the new sticky position block support, and is a follow-on from https://github.com/WordPress/wordpress-develop/pull/3865
Backports the PHP parts of the following PRs:
- https://github.com/WordPress/gutenberg/pull/46142
- https://github.com/WordPress/gutenberg/pull/47626
- https://github.com/WordPress/gutenberg/pull/47665
Notes on this backport:
- The backport deviates from Gutenberg trunk in one way:
fixed
position type is excluded from the appearance tools opt-in: this is because fixed positioning isn't being opted-in for the Group block and isn't ready to be used as a default opt-in. The code paths exist forfixed
positioning as it will be supported in the future, but for now,sticky
is the only one exposed by default. (The overall block support should still support it as a valid value, though) - The styling output while logged in depends on the JS packages update, as it will include the CSS changes in #47665, so while testing this PR you might notice an odd styling issue while logged in. This should be resolved once the package update lands.
- With the above caveat shouldn't really be an issue though, as there is no way for a user to set a group block to sticky without the JS packages landing. This PR is to land the required PHP changes in advance of that.
Screenshots:
| While logged in (CSS variable that's included in JS packages update isn't yet output in this PR, once it is the sticky block will sit below the admin bar) | While not logged in (correct) |
| --- | --- |
| | |
Testing instructions:
- In the Group block's
block.json
file add a"position": { "sticky": true }
object under"supports"
(Note: for this change to propagate in your testing, you might need to rebuild the block JSON cache locally). - Create a post and add a bunch of paragraphs.
- Insert the below markup at the beginning of the post (this is Group block markup with the appropriate position style object to sticky to top).
- Publish the post, and on the site frontend, the group block should stick to the top of the screen once it has been scrolled past the top of the screen (the block sticks to the scrollable area of its container)
Group block markup for testing purposes:
{{{html
<div class="wp-block-group has-background" style="background-color:#f2aeae;padding-top:1em;padding-right:1em;padding-bottom:1em;padding-left:1em">
<p>A paragraph in a sticky group</p>
</div>
}}}
Trac ticket:
@andrewserong commented on PR #3973:
20 months ago
#10
Thanks for reviewing @felixarntz!
Does this have to wait for the JS package update, or can it be committed to core before already?
This can be committed before the JS package update, since this PR doesn't opt any of the blocks into the new control, so it doesn't expose anything to a user. I think it's preferable to land this PR first, so that when the JS packages update happens, the PHP parts will already be in place.
@davidbaumwald commented on PR #3973:
20 months ago
#11
Merged into core in https://core.trac.wordpress.org/changeset/55285.
Even if the use of an empty
dd
element is ok for the validator, the use of a list of definitions with an emptydd
element sounds strange to me.I wonder if we shouldn't simply use an unordered list here.
By the way, I'm ok that the empty
dd
is a good first way to fixe the validation error.