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

Basic image alignment options #198

Closed
koke opened this issue Oct 26, 2018 · 17 comments · Fixed by #1460
Closed

Basic image alignment options #198

koke opened this issue Oct 26, 2018 · 17 comments · Fixed by #1460
Labels
Aztec Parity Feature exists in Aztec but not yet in Gutenberg mobile Blocks

Comments

@koke
Copy link
Member

koke commented Oct 26, 2018

The image block (and others) have an alignment toolbar that we need to implement

screen shot 2018-10-17 at 12 41 27

Edit: The theme-based options are now tracked via #1565.

There's a good amount of conversation about this in #169, but to summarize:

  • The relatively easy part is to add the alignment toolbar and hint the selected setting by tweaking the image margins and alignment. This is what Aztec does already.
  • If we want to wrap text around the image for left/right floats, that's much trickier and we're not worrying about that just yet.
  • What alignment options are available depend on the theme, so we'll need to research how we'll get that information.

We'll want the first item for the beta (three days), but we might want to consider the other two depending on research.

@koke koke added the Blocks label Oct 26, 2018
@koke koke added this to the Beta milestone Oct 26, 2018
@koke koke mentioned this issue Oct 26, 2018
2 tasks
@koke koke mentioned this issue Jan 16, 2019
2 tasks
@SergioEstevao
Copy link
Contributor

Shouldn't we split this work for each block? I see at least the following sub tasks:

  • Alignment toolbar UI implementation, what to do regarding shorter screens and how to condense interface, on the web it's done with a pop up.
  • Implement alignment for rich text based blocks
  • Alignment for media blocks, image and video (is it needed for video?)

@koke
Copy link
Member Author

koke commented May 31, 2019

It makes sense to split the tasks that way but I wouldn’t create single issues until someone starts working on them

@timnolte
Copy link

Not having alignment right now for image placement is a big pain. My assumption here is that most people "aren't effected" because their theme drives this perhaps. The theme I use gives me full control over image alignment.

@thehenrybyrd
Copy link

We've had a request in #2191084-zen for text alignment options, specifically to justify text.

@thehenrybyrd
Copy link

Here's another for justifying text in #2195624-zen.

I'm happy with using the app if you can help me to justify my text please.

@hypest hypest changed the title Block alignment options Image alignment options Aug 6, 2019
@hypest hypest removed this from the Beta milestone Aug 6, 2019
@hypest
Copy link
Contributor

hypest commented Aug 6, 2019

The relatively easy part is to add the alignment toolbar and hint the selected setting by tweaking the image margins and alignment. This is what Aztec does already.

Let's prioritize the Aztec-parity bit and have it in the Open Beta.

@mchowning mchowning self-assigned this Aug 8, 2019
@mchowning
Copy link
Contributor

mchowning commented Aug 9, 2019

The relatively easy part is to add the alignment toolbar and hint the selected setting by tweaking the image margins and alignment. This is what Aztec does already.

The platforms actually appear to have some pretty different behavior on Aztec with respect to image size and alignment (I'm not super familiar with Aztec, so please let me know if you think I might be mistaken).

iOS Android
Aligns the image if the image does not take up the full width of the screen. In other words, any reasonably large image must have a size of either thumbnail or medium, as opposed to large or full, for there to be any visible alignment on the image (images that are small are never stretched). Always shows the image with the full width of the screen regardless of the selected size. Even a small image that is set to thumbnail size is stretched to fill the screen's width. The only way Android indicates the alignment of the image is by aligning the image's caption (if it has one).
Screen Shot 2019-08-09 at 11 07 22 AM Screen Shot 2019-08-09 at 11 03 57 AM

My take is that for the beta release we should aim for the iOS experience because it is significantly better:

  • Images that should not take up the full screen (whether due to the size attribute or the image's actual size) do not take up the full screen width and reflect any applicable alignment.
  • Larger images take up the full screen width (with appropriate padding) and do not reflect any applicable alignment.
  • One additional issue is that image blocks can also have wide alignment, which is not supported in Aztec. I suggest that for the open beta we only support wide alignment to the extent that the user can set that alignment and the UI renders image blocks with that alignment, but we do not indicate the alignment in the UI (so the UI would be the same as an image with default or full alignment).

Also, this issue appears to overlap with #1263. @hypest, should this issue be closed in favor of that one? Or we could use #1263 for adding the controls and this issue for updating the UI to appropriately reflect the alignment in the UI since the discussion in this card and #169 has focused on the UI presentation?

cc: @hypest , @iamthomasbishop

@hypest
Copy link
Contributor

hypest commented Aug 9, 2019

should this issue be closed in favor of that one?

Let's keep both for now, mainly because there's good content in this one and the #1263 is more overarching.

@mchowning mchowning removed their assignment Aug 9, 2019
@hypest hypest added the Aztec Parity Feature exists in Aztec but not yet in Gutenberg mobile label Sep 14, 2019
@mchowning mchowning self-assigned this Sep 20, 2019
@mchowning
Copy link
Contributor

@iamthomasbishop left some design feedback regarding alignment here:

I'd leave alignment out of the settings sheet, and delegate that to the Quick Toolbar buttons. I would start by displaying the alignment options inline, and if we have time to, we should consider collapsing those into a single dropdown button. If that happens, let's have a discussion around what that interaction looks like, because we haven't introduced any dropdown buttons yet.

@hypest
Copy link
Contributor

hypest commented Sep 23, 2019

For the upcoming Open Beta target, let's only include the "Aztec parity" subset of the feature.

@mchowning
Copy link
Contributor

👋 @iamthomasbishop . For an initial implementation where the alignment buttons are not collapsed. Do you think it would be better to put them to the left or the right of the pencil/edit icon. I'm working on this now and (for no particular reason) I currently have the alignment icons to the right:

Screen Shot 2019-10-01 at 5 57 21 PM

@timnolte
Copy link

timnolte commented Oct 1, 2019

On the desktop block editor the icons show up to the right of the block actions menu. So I would say to the right seems to align. Now I would say that it could be something that actually shows after click the pencil as part of the other block edit options.

@iamthomasbishop
Copy link
Contributor

I agree with @timnolte regarding positioning (after pencil icon), I don’t see a reason why we should diverge from core/web here.

Side note: we should switch away from the pencil icon to the new “replace” icon that core is now using on the image block – much clearer. Obviously that’s a separate issue but the sooner we can do this the better 😄

@mchowning
Copy link
Contributor

we should switch away from the pencil icon to the new “replace” icon that core is now using on the image block – much clearer. Obviously that’s a separate issue but the sooner we can do this the better 😄

Is this the "replace" icon you would like @iamthomasbishop ?

Screen Shot 2019-10-02 at 5 22 36 PM

If it is, I'll include that update in my changes.

@iamthomasbishop
Copy link
Contributor

That's the one! Still not a great solution imo, but it at least is better and matches Core. Can we also apply it to the other blocks that contain the icon (image, video, maybe others?) if it's not a hassle? (again, feel free to break that out into a separate issue if easier)

@mchowning
Copy link
Contributor

Hey @hypest ! This card was automatically closed with my PR adding left, right, and center image alignment options for Aztec-parity, but I have not added support for wide or full alignment handling. Would you prefer to re-open this card or create a separate card for that remaining work?

@mchowning mchowning removed their assignment Oct 28, 2019
@hypest
Copy link
Contributor

hypest commented Nov 11, 2019

👋 @mchowning , there's been minimal discussion in this ticket about the theme-controlled size options so, let's keep this ticket closed and create a new one only for them. Here's the new one: #1565. Will revise the title and description in the current one.

@hypest hypest changed the title Image alignment options Basic image alignment options Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aztec Parity Feature exists in Aztec but not yet in Gutenberg mobile Blocks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants