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 Reverse on mobile attribute for Media & Text Blocks #56279

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

masperber
Copy link

What?

Fixes #26215

Adds an attribute Reverse on mobile to the Media & Text Blocks. When selected, the block's column order will be reversed if the screen is narrower than the block's mobile breakpoint.

For Media & Text Blocks, if 'Stacked on mobile' is also selected, then the content will appear on top and the media will appear on bottom, because default behavior is for media to appear on top and content to appear on bottom when 'Stacked on mobile' is selected. If 'Stacked on mobile' is not selected, then 'Reverse on mobile' will simply reverse the column order for mobile views.

Why?

This PR is being submitted along with #55763 because Columns Blocks and Media & Text Blocks are frequently used to display media and content in a zig-zag pattern. For example:

[image]   [text ]
[text ]   [image]
[image]   [text ]

The Media & Text Block is coded to stack content with the media on top in the mobile view, like this:

[image]
[text ]
[image]
[text ]
[image]
[text ]

However, some users may wish to have the text stacked on top in the mobile view, like this:

[text ]
[image]
[text ]
[image]
[text ]
[image]

This PR adds the option to reverse the column order on mobile if desired, giving users more flexibility over how responsive content translates to the site's mobile view.

How?

The Reverse on mobile attribute allows users greater flexibility in determining how their content is translated from unstacked to stacked. Users can choose the order in which the stacked content will appear.

Testing Instructions

  1. Open a post or page.
  2. Add at least 3 or 4 Media & Text Blocks.
  3. Add media and content in a zig-zag pattern, as described above.
  4. Apply the "Reverse on mobile" attribute to every Media & Text Block.
  5. Observe the stacking behavior in the editor both with and without the attribute toggled.

Testing Instructions for Keyboard

  1. Insert a Media & Text Block using existing methods.
  2. Select the block using existing methods.
  3. Open the Settings sidebar using existing methods.
  4. Use the Tab key to highlight Reverse on mobile, and use Space to toggle the attribute.

Screenshots or screencast

Screenshot 2023-11-17 at 3 11 55 PM

Screenshot showing the location of the 'Reverse on mobile' attribute in the editor, along with Columns blocks displaying the aforementioned 'zig-zag' pattern.

Screenshot 2023-11-17 at 3 12 50 PM

Screenshot showing the appearance of stacked Media & Text Blocks in the editor. The 'Reverse on mobile' attribute has been applied to every Media & Text Block so that the 'zig-zag' pattern translates to a 'text on top' pattern.

Screenshot 2023-11-17 at 3 13 08 PM

Screenshot showing the front end of a page with stacked Media & Text Blocks, with every Media & Text Block having the 'Reverse on mobile' attribute.

Adds a 'Reverse on mobile' attribute for Media & Text Blocks with a ToggleControl added to the block settings. When selected, the block's display is reversed for mobile views.
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 17, 2023
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @masperber! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@masperber masperber changed the title Add 'Reverse on mobile' attribute for Media & Text Blocks Add Reverse on mobile attribute for Media & Text Blocks Nov 17, 2023
@carolinan carolinan added [Type] Enhancement A suggestion for improvement. [Block] Media & Text Affects the Media & Text Block labels Nov 21, 2023
@carolinan
Copy link
Contributor

Hi @masperber. Thank you for working on this.
The new feature works as described. Unfortunately it re-introduces an accessibility problem where the tabbing order no longer follows the visual order. Meaning, that for anyone who uses the keyboard to navigate, the focus will move up and down the page in the wrong order. This is a problem with using CSS only to move elements.

To test:

Add two media and text blocks, enable the new setting on the second block.
Add links to both content areas.
Add links to both blocks using the link option in the toolbar.
Save and view the blocks on the front.

Reduce the browser window width to trigger the reversal.
Press tab until focus is on the first linked image.
Press tab. The link in the content area is selected.

Press tab again.
Now this is the problem: In the second block, which is reversed,
the image is now in focus even though it is below the text content.
Press tab again. Focus now jumps up the page again to the text content above the image.
This is problematic for people who use screen magnifiers, because it is unexpected and makes it difficult to navigate the page.

@masperber
Copy link
Author

Hey @carolinan , thanks for testing!

I followed the testing steps that you provided, and I was unable to reproduce the issue.

Here is a video showing the tab following the visual order in the Chrome browser:

Media-Text.Tabs.on.Chrome.mp4

Here is a video showing the tab following the visual order in the Edge browser:

Media-Text.Tabs.on.Edge.mp4

Can you give me any more details about your testing environment so that I might be able to reproduce what you observed?

@carolinan
Copy link
Contributor

I am using Chrome on mac.
Both blocks are identical except the second block has the reverse option enabled. The media is on the left.

@masperber
Copy link
Author

@carolinan I was able to reproduce the issue you described only when Media is on the left and the reverse attribute is selected.

With more testing, I discovered that the issue actually already exists on core when Media is on the right.

Steps to reproduce:

  • Make at least 1 Media & Text Block, with Media on the right.
  • Add a link to the content area
  • Add a link to the media
  • View the page, and set the page width narrower than the mobile breakpoint
  • Tab until you read the Media & Text elements
  • The tab will reach the link in the content area before it reaches the media, despite the fact that the media appears before the content area in the visual order.

In fact, if you set up a Media & Text Block with Media on the right and choose the Reverse on mobile attribute that is added by this PR, then the issue is resolved, and the tab order follows the visual order.

With all of that in mind, I think that the tab order not following the visual order is a separate issue and should not be considered when discussing the merits of this PR.

@carolinan
Copy link
Contributor

I still don't believe that using CSS only is the right solution. To have the media on left is the default and we shouldn't knowingly introduce more issues.
Can we explore re-ordering the elements with JavaScript?

@carolinan
Copy link
Contributor

@masperber
Copy link
Author

I still don't believe that using CSS only is the right solution. To have the media on left is the default and we shouldn't knowingly introduce more issues. Can we explore re-ordering the elements with JavaScript?

I think you’re right, but since CSS is what is currently used to reorder the elements when ‘Stack on mobile’ is enabled, I don’t think that solving this problem is within the scope of this PR.

As I mentioned, this PR actually solves the current issue where tab order doesn’t follow visual order when ‘Stack on mobile’ is selected and media is on the right, so I don’t think it’s correct to say that this PR introduces a new issue.

@carolinan
Copy link
Contributor

If I read your description correctly it doesn't actually solve it unless you also enable the new option, and that is not a fix that will be suitable for everyone.

@masperber
Copy link
Author

For the last test to pass, a deprecation of the version without the new setting needs to be added: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/media-text/deprecated.js

https://developer.wordpress.org/block-editor/reference-guides/block-api/block-deprecation/

Thanks for calling this out. I can work on the deprecation, but I won’t be able to push any commits until 2 weeks from now.

@t-hamano
Copy link
Contributor

Thank you for working on this issue, @masperber!

I think that the visual order and focus order of media and content do not match, which is certainly a problem from an accessibility perspective. Referring to the MDN documentation, it is mentioned that the grid and order properties should only be used for visual sorting, not logical sorting.

Since the media in the Media and Text block is also part of the content, the "Reverse on mobile" setting seems to add a new logical sorting.

However, as mentioned in this comment, trunk already does this logical sorting. I think it would be a good idea to resolve the issue with trunk first and then try again with this PR.

In any case, I think this PR needs feedback on accessibility, so I'd like to send a ping to @alexstine, @joedolson, @afercia.

@t-hamano t-hamano added the Needs Accessibility Feedback Need input from accessibility label Nov 23, 2023
@masperber
Copy link
Author

@t-hamano thanks for this feedback. I agree with the feedback, but it looks like this issue already exists on trunk when ‘Stack on mobile’ is selected and Media is on the right:

Steps to reproduce on trunk:

  • Make at least 1 Media & Text Block, with Media on the right.
  • Add a link to the content area
  • Add a link to the media
  • View the page, and set the page width narrower than the mobile breakpoint
  • Tab until you read the Media & Text elements
  • The tab will reach the link in the content area before it reaches the media, despite the fact that the media appears before the content area in the visual order.

So, while I agree that there is an accessibility issue, said accessibility issue is already present on trunk, so I don’t think that should be a blocker to merging this PR.

@afercia
Copy link
Contributor

afercia commented Nov 24, 2023

With all of that in mind, I think that the tab order not following the visual order is a separate issue and should not be considered when discussing the merits of this PR.

I'd agree with @carolinan that using CSS only doesn't seem ideal.
Basically, all the CSS properties that alter the visual order should be banned when they impact logical order, meaning, functionality, and Tab order. All the grid techniquest that alter order, all the reverse and order properties should only be used for 'decorative' changes that not impact what mentioned above.

The fact the problem already exists, in part, isn't a good argument IMHO to add one more instance yet of mismatch between visual and DOM order. It would only add more technical debt that would need to be fixed in the future anyways.

@carolinan
Copy link
Contributor

The reason why it has not been attempted with JavaScript yet, is that there has been a consensus that loading JavaScript on the front should be avoided; but with the introduction of the interactivity API, that doesn't seem to be the case anymore.

@t-hamano
Copy link
Contributor

Regarding the problem we are experiencing with trunk, mentioned in this comment, I think we need to explore the ideal approach separately.

Therefore, I have reopened #38537.

@cbirdsong
Copy link

cbirdsong commented Nov 29, 2023

Has duplicating the image markup been considered? That would be a Javascript-free solve for this and the existing issue with media on the right.

The markup would look like this:

  • .wp-block-media-text__media
  • .wp-block-media-text__content
  • copy of .wp-block-media-text__media with hidden attribute applied

From here, CSS could override the default hidden/visible settings based on the classes applied for moving the image to the other side and reversing the order on mobile.

@masperber
Copy link
Author

@cbirdsong Very clever! I have an idea of how this could be implemented, and I should be able to push a commit in about 8 to 10 days. It’s possible this could also resolve #38537

@t-hamano
Copy link
Contributor

t-hamano commented Dec 1, 2023

Has duplicating the image markup been considered? That would be a Javascript-free solve for this and the existing issue with media on the right.

I think this is a very good approach, but the same approach may not be applicable in #55763, which adds functionality similar to this PR. to the Columns block.

  • A column block does not necessarily have only two columns.
  • For example, if there is a function that creates a table of contents from the heading elements contained in the content, the duplicated and hidden content may also be read.
  • If some feature is processing content depending on its content length, hidden content may have an impact.

@t-hamano
Copy link
Contributor

t-hamano commented Dec 8, 2023

I discovered that a similar PR had been submitted in the past, leading to in-depth discussions about accessibility.

See: #40967

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Needs Accessibility Feedback Need input from accessibility [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Media & Text Block: Allow reversed stacking order on mobile.
5 participants