-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
base: trunk
Are you sure you want to change the base?
Add Reverse on mobile attribute for Media & Text Blocks #56279
Conversation
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.
👋 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. |
Hi @masperber. Thank you for working on this. To test:Add two media and text blocks, enable the new setting on the second block. Reduce the browser window width to trigger the reversal. Press tab again. |
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.mp4Here is a video showing the tab following the visual order in the Edge browser: Media-Text.Tabs.on.Edge.mp4Can you give me any more details about your testing environment so that I might be able to reproduce what you observed? |
I am using Chrome on mac. |
@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:
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. |
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. |
For the last test to pass, a deprecation of the version without the new setting needs to be added: https://developer.wordpress.org/block-editor/reference-guides/block-api/block-deprecation/ |
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. |
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. |
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. |
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 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:
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. |
I'd agree with @carolinan that using CSS only doesn't seem ideal. 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. |
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. |
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. |
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:
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. |
@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 |
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.
|
I discovered that a similar PR had been submitted in the past, leading to in-depth discussions about accessibility. See: #40967 |
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:
The Media & Text Block is coded to stack content with the media on top in the mobile view, like this:
However, some users may wish to have the text stacked on top in the mobile view, like this:
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
Testing Instructions for Keyboard
Screenshots or screencast
Screenshot showing the location of the 'Reverse on mobile' attribute in the editor, along with Columns blocks displaying the aforementioned 'zig-zag' pattern.
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 showing the front end of a page with stacked Media & Text Blocks, with every Media & Text Block having the 'Reverse on mobile' attribute.