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

Try: Improve floats UI. #10085

Merged
merged 2 commits into from Oct 6, 2018

Conversation

Projects
None yet
5 participants
@jasmussen
Contributor

jasmussen commented Sep 21, 2018

This PR tries to improve the block UI on floated elements. It does a few things:

  • It hides the side UI entirely. Because this is now just the ellipsis and drag handle, this is not the end of the world. The argument being, that when you need to move an image up or down, you do it before you float. And the side UI overlapped with parents anyway, so the only alternative was to use the mobile UI (buttons below the block), for floats. This was not as compelling as initially thought.
  • Because the block outline is hidden on floated UI anyway (it has to be, because it doesn't know how tall the floated content is due to the nature of floats), the toolbar looked offset to the side. This PR positions the block toolbar to better align with the float.
  • Improves the mobile UI for floats, it was quite broken.
  • It adds a min-width to embeds, which after the most recent responsive feature would collapse completely when floated.

Screenshot. No side UI:

no side ui

Improved mobile UI:

improved mobile positioning

Min width for embeds:

min width for embeds

Right floated toolbar positioning:

positioning for right floated

Left floated toolbar positioning:

positioning for left floated

@jasmussen jasmussen added this to the 4.0 milestone Sep 21, 2018

@jasmussen jasmussen self-assigned this Sep 21, 2018

@jasmussen jasmussen requested a review from WordPress/gutenberg-core Sep 21, 2018

@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Sep 21, 2018

Contributor

This would also close #7492.

Contributor

chrisvanpatten commented Sep 21, 2018

This would also close #7492.

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Sep 21, 2018

Contributor

It feels like bad UX to me to make it impossible to move a floated block. Perhaps if there were keyboard shortcuts for moving blocks I would not mind, but I wish there was some way to still move the floated blocks. In the Classic Editor, you would just click and drag an image to move it. Is there any reason why the same behavior could not be implemented in Gutenberg?

Contributor

ZebulanStanphill commented Sep 21, 2018

It feels like bad UX to me to make it impossible to move a floated block. Perhaps if there were keyboard shortcuts for moving blocks I would not mind, but I wish there was some way to still move the floated blocks. In the Classic Editor, you would just click and drag an image to move it. Is there any reason why the same behavior could not be implemented in Gutenberg?

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 24, 2018

Contributor

It feels like bad UX to me to make it impossible to move a floated block.

I don't necessarily disagree. But I also strongly subscribe to the idea of iteration. In that vein, this feels like a step forward compared to what's in 3.9, and also a step that doesn't preclude further improvements later on.

Drag and drop is, on principle, someting we consider an additive enhancement on top of other explicit mover controls. That is to say — it's not that we can't do it, it just has to be in addition to other features.

I do really like the idea of shortcuts for moving up or down, haven't we a separate ticket for this?

Contributor

jasmussen commented Sep 24, 2018

It feels like bad UX to me to make it impossible to move a floated block.

I don't necessarily disagree. But I also strongly subscribe to the idea of iteration. In that vein, this feels like a step forward compared to what's in 3.9, and also a step that doesn't preclude further improvements later on.

Drag and drop is, on principle, someting we consider an additive enhancement on top of other explicit mover controls. That is to say — it's not that we can't do it, it just has to be in addition to other features.

I do really like the idea of shortcuts for moving up or down, haven't we a separate ticket for this?

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Sep 24, 2018

Contributor

@jasmussen Fair enough.

I do really like the idea of shortcuts for moving up or down, haven't we a separate ticket for this?

If it exists, I can't find it.

Contributor

ZebulanStanphill commented Sep 24, 2018

@jasmussen Fair enough.

I do really like the idea of shortcuts for moving up or down, haven't we a separate ticket for this?

If it exists, I can't find it.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 24, 2018

Contributor

Here it is: #71

If we can think up a good keyboard shortcut for "move up" an "move down" we can create a new specific ticket for that. Should be easy.

Contributor

jasmussen commented Sep 24, 2018

Here it is: #71

If we can think up a good keyboard shortcut for "move up" an "move down" we can create a new specific ticket for that. Should be easy.

@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Sep 24, 2018

Contributor

Because the block outline is hidden on floated UI anyway (it has to be, because it doesn't know how tall the floated content is due to the nature of floats)

I'm pretty curious about this. I was playing around with this recently and didn't see any notable issues with having a block outline for floated content:

edit_page_ tomodomo _wordpress

Is there any context on this in a previous ticket?

Contributor

chrisvanpatten commented Sep 24, 2018

Because the block outline is hidden on floated UI anyway (it has to be, because it doesn't know how tall the floated content is due to the nature of floats)

I'm pretty curious about this. I was playing around with this recently and didn't see any notable issues with having a block outline for floated content:

edit_page_ tomodomo _wordpress

Is there any context on this in a previous ticket?

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 25, 2018

Contributor

Is there any context on this in a previous ticket?

First off, I'm not against floats having outlines. If we can make it work, PRs will be fast-tracked ;)

Although I can't find the PR now (looked in my closed PRs because I'm pretty sure I removed them), I THINK it's related to the overall float refactors, which now use this method: https://codepen.io/joen/pen/YOZRWP

The overall idea is: the float container is not floated, but the figure inside is floated. Because by keeping the float container the same width as the main column. we can ensure that the figure floated inside aligns itself to this very main column, and allowing the main column to be constrained only by a max-width. The float container then collapses to zero pixels height, so that it's invisible. But it's still there to constrain the float to match the main column width.

By doing all that we can trivially add wide and fullwide images by simply removing the max-width on those blocks, and we don't have to resort to crazy vw units or calc statements.

Put it differently: if you can make it work with boundaries also on the frontend in the gutenberg starter theme or https://github.com/jasmussen/navi, then 👍 👍

Contributor

jasmussen commented Sep 25, 2018

Is there any context on this in a previous ticket?

First off, I'm not against floats having outlines. If we can make it work, PRs will be fast-tracked ;)

Although I can't find the PR now (looked in my closed PRs because I'm pretty sure I removed them), I THINK it's related to the overall float refactors, which now use this method: https://codepen.io/joen/pen/YOZRWP

The overall idea is: the float container is not floated, but the figure inside is floated. Because by keeping the float container the same width as the main column. we can ensure that the figure floated inside aligns itself to this very main column, and allowing the main column to be constrained only by a max-width. The float container then collapses to zero pixels height, so that it's invisible. But it's still there to constrain the float to match the main column width.

By doing all that we can trivially add wide and fullwide images by simply removing the max-width on those blocks, and we don't have to resort to crazy vw units or calc statements.

Put it differently: if you can make it work with boundaries also on the frontend in the gutenberg starter theme or https://github.com/jasmussen/navi, then 👍 👍

@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Sep 29, 2018

Contributor

@jasmussen happy to take this into slack but with the block outlines it shouldn’t really matter on the front-end, right? Or do you mean because of a potential collision with editor styles?

Contributor

chrisvanpatten commented Sep 29, 2018

@jasmussen happy to take this into slack but with the block outlines it shouldn’t really matter on the front-end, right? Or do you mean because of a potential collision with editor styles?

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 30, 2018

Contributor

happy to take this into slack but with the block outlines it shouldn’t really matter on the front-end, right? Or do you mean because of a potential collision with editor styles?

That's right, but the way that we paint the block outlines right now, I can't think of a way to paint them around the floats, the way we do them.

But that doesn't mean it's not possible, and to be clear I'm not against them being there at all!

Contributor

jasmussen commented Sep 30, 2018

happy to take this into slack but with the block outlines it shouldn’t really matter on the front-end, right? Or do you mean because of a potential collision with editor styles?

That's right, but the way that we paint the block outlines right now, I can't think of a way to paint them around the floats, the way we do them.

But that doesn't mean it's not possible, and to be clear I'm not against them being there at all!

Try: Improve floats UI.
This PR tries to improve the block UI on floated elements. It does a few things:

- It hides the side UI entirely. Because this is now just the ellipsis and drag handle, this is not the end of the world. The argument being, that when you need to move an image up or down, you do it before you float. And the side UI overlapped with parents anyway, so the only alternative was to use the mobile UI (buttons below the block), for floats. This was not as compelling as initially thought.
- Because the block outline is hidden on floated UI anyway (it has to be, because it doesn't know how tall the floated content is due to the nature of floats), the toolbar looked offset to the side. This PR positions the block toolbar to better align with the float.
- Improves the mobile UI for floats, it was quite broken.
- It adds a min-width to embeds, which after the most recent responsive feature would collapse completely when floated.
@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Oct 1, 2018

Contributor

Rebased & Squashed.

Contributor

jasmussen commented Oct 1, 2018

Rebased & Squashed.

@mcsf

mcsf approved these changes Oct 6, 2018

I agree we should keep the momentum. Let's get this in and iterate!

@mcsf mcsf merged commit 7ec834b into master Oct 6, 2018

2 checks passed

codecov/project 49.44% (+0.87%) compared to 71fc9bb
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mcsf mcsf deleted the try/better-floats-ui branch Oct 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment