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 Columns Blocks #55763

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

masperber
Copy link

@masperber masperber commented Nov 1, 2023

What?

Fixes #40706

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

For Columns Blocks, this means the column order will simply be reversed.

Why?

This PR is being submitted along with #56279 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 ]

For this type of pattern, users typically want the content to stack on mobile like this:

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

With the current Columns Block behavior, the content ends up looking like this:

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

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 Columns Blocks, with 2 columns each.
  3. Add media and content in a zig-zag pattern, as described above.
  4. Apply the "Reverse on mobile" attribute to every other Columns Block.
  5. Observe the stacking behavior in the editor both with and without the attribute toggled.
  6. Repeat the previous steps with the Media & Text blocks, except apply the "Reverse on mobile" attribute to all blocks.

Testing Instructions for Keyboard

  1. Insert a Columns 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 12 48 52 AM

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 12 50 53 AM

Screenshot showing the appearance of stacked Columns blocks in the editor. The 'Reverse on mobile' attribute has been applied to every other Columns Block so that the 'zig-zag' pattern translates to an appropriate stacked pattern.

Screenshot 2023-10-31 at 10 11 15 PM

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

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 1, 2023
Copy link

github-actions bot commented Nov 1, 2023

👋 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.

@stephymiehle
Copy link
Contributor

I love the idea and the implementation looks clean, but I'm not sure that I'd think of this as a style; it feels more like an on/off toggle.

I don't have any custom Columns styles but have toyed with options for Media & Text and would guess that others may have too :) If I've chosen a custom style on a Block, I'd still like to have the option to reverse them, and we can only choose one style.

@MadtownLems
Copy link

I love the idea and the implementation looks clean, but I'm not sure that I'd think of this as a style; it feels more like an on/off toggle.

Could not agree more. Styles are very limiting in that you can only have one. This definitely feels like it should be an on/off toggle.

Not only would that allow it to work alongside other Styles - but it just doesn't really "feel" like a style, either.

@masperber
Copy link
Author

@MadtownLems @blindingstars Yes, I agree with the feedback that this feels more appropriate as an on/off toggle that should appear under the "Stacked on Mobile" option when that option is selected. That is how I originally intended to implement it, but there was a knowledge gap on my end as far as how to add a toggle. I was successfully able to research and learn how to add a style, so that's what I ended up going with.

Can either of you point me to a resource where I could learn about how to implement a conditionally visible on/off toggle in block settings? If so, I'd be happy to refactor the implementation as a toggle.

@MadtownLems
Copy link

This is also out of my area of development knowledge, but I'm wondering if you can clarify this point, so that someone else might be able to better point you in the right direction.

conditionally visible on/off toggle in block settings

What do you mean by conditionally visible? Why wouldn't it always be visible on Columns or Media & Text Blocks? What are the 'conditions' you're talking about?

@ndiego
Copy link
Member

ndiego commented Nov 15, 2023

For this type of implementation, I would explore something similar to the toggle in the Media & Text block:

image

The code for this can be found here. Note that this will require an additional attribute to the Columns block.

@masperber
Copy link
Author

What do you mean by conditionally visible? Why wouldn't it always be visible on Columns or Media & Text Blocks? What are the 'conditions' you're talking about?

Thanks for asking for clarity. What I mean is that the "Reverse on mobile" toggle would only appear if the "Stack on mobile" toggle is selected.

@stephymiehle
Copy link
Contributor

@masperber Hmm, I see where you're coming from about the conditional part – but is that actually a requirement? If we don't need that conditional check, maybe this would be an easier lift for the toggle.

For instance, what if someone had a bunch of Media & Text that they wanted zig-zagged on desktop, but all image-to-the-left on mobile? I can't really imagine why someone would want to reverse unstacked Columns on mobile, but your existing code would accommodate that. With an additional minor update to packages/block-library/src/media-text/style.scss, this could also accommodate the unstacked Media & Text version.

@MadtownLems
Copy link

For more insights into handling the conditional part, you could look at the following core blocks:

Group Block -> When you enable "Inner blocks use content width", the inputs for width values and justification show up below it.

Image Block -> When you set an aspect ratio other than Original, the Scale control shows up below it.

Based on community feedback, changes the Reverse on mobile style to an attribute with a ToggleControl.
@masperber masperber changed the title Add Reverse on Mobile style for Columns and Media & Text Blocks Add Reverse on Mobile Attribute for Columns and Media & Text Blocks Nov 17, 2023
@masperber
Copy link
Author

Based on community feedback, I've refactored the PR to add 'Reverse on mobile' as an attribute with a ToggleControl instead of a style. I've edited the initial post with updated screenshots. The front-end functionality is the same as before.

Fixes a bug where Media & Text Blocks did not reverse when not stacked on mobile.
Fixes a bug where Columns Blocks stacked on mobile when Stack on mobile was not selected and Reverse on mobile was selected
@rodolfomartinez
Copy link

This is just what I need :-) Any ETA for when this will be live?

@ndiego
Copy link
Member

ndiego commented Nov 17, 2023

@masperber while the functionality is the same between the Media & Text blocks and Columns blocks, I would break these into two separate PRs, one for each block. That will make the PR easier to evaluate and merge.

@masperber
Copy link
Author

@rodolfomartinez Thanks for your feedback! I'm glad you like it. :)

I'm not sure how long it will take to be discussed and approved. In the meantime, feel free to use this plugin that I made which adds these features as a style.

@ndiego Sure thing, I'll adjust this PR to make the change only for Columns Blocks, and then I'll make a separate PR for Media & Text Blocks, which I will link here.

@rodolfomartinez
Copy link

awesome, thank you so much!

@rodolfomartinez
Copy link

Might have found a bug - should I create the issue here or on the plugin page?

This commit removes the Reverse on mobil attribute from this PR so that it can be included in a separate PR.
@masperber masperber changed the title Add Reverse on Mobile Attribute for Columns and Media & Text Blocks Add Reverse on Mobile Attribute for Columns Blocks Nov 17, 2023
@masperber
Copy link
Author

@ndiego I've removed the attribute from the Media & Text Block in this PR.

@rodolfomartinez feel free to push your own commit to fix it, or let me know and I can fix it.

@rodolfomartinez
Copy link

Not exactly sure how to fix it. On Safari if I reverse, the image stays off to the right. (it never really stacks, just kind of floats off to the side)

@masperber
Copy link
Author

Not exactly sure how to fix it. On Safari if I reverse, the image stays off to the right. (it never really stacks, just kind of floats off to the side)

@rodolfomartinez is this in regards to the plugin that I linked, or to this PR? If it's in regards to the plugin, can you add an Issue on that repository?

@t-hamano
Copy link
Contributor

t-hamano commented Nov 23, 2023

I commented here about accessibility concerns in #56279, which makes similar changes to media and text blocks.

I think this PR will also require accessibility feedback.

@t-hamano t-hamano added [Type] Enhancement A suggestion for improvement. Needs Accessibility Feedback Need input from accessibility [Block] Columns Affects the Columns Block labels Nov 23, 2023
@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] Columns Affects the Columns 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.

Columns Should be Able to Set to Stack Left on Top OR Right on Top on Mobile
6 participants