Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#30147 closed enhancement (fixed)

Consider adding alignment buttons inline for an image

Reported by: iseulde's profile iseulde Owned by: azaozz's profile azaozz
Milestone: 4.1 Priority: normal
Severity: normal Version:
Component: TinyMCE Keywords: has-patch
Focuses: ui, javascript Cc:

Attachments (12)

30147.patch (17.9 KB) - added by iseulde 9 years ago.
Screen Shot 2014-11-03 at 20.05.15.png (38.3 KB) - added by iseulde 9 years ago.
30147.2.patch (17.4 KB) - added by azaozz 9 years ago.
30147.3.patch (17.5 KB) - added by azaozz 9 years ago.
Remove unused var.
30147.4.patch (19.6 KB) - added by azaozz 9 years ago.
img-toolbar-1.png (204.5 KB) - added by azaozz 9 years ago.
img-toolbar-2.png (205.3 KB) - added by azaozz 9 years ago.
img-toolbar-3.png (157.2 KB) - added by azaozz 9 years ago.
image1.png (172.0 KB) - added by azaozz 9 years ago.
Screenshot_2014-11-12-12-31-00.png (164.0 KB) - added by azaozz 9 years ago.
Screenshot_2014-11-12-12-31-20.png (134.5 KB) - added by azaozz 9 years ago.
30147.triangle-close-right-edge.patch (852 bytes) - added by afercia 9 years ago.

Download all attachments as: .zip

Change History (38)

This ticket was mentioned in Slack in #feature-imageflow by janneke. View the logs.


9 years ago

This ticket was mentioned in Slack in #feature-imageflow by janneke. View the logs.


9 years ago

@iseulde
9 years ago

#3 @iseulde
9 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.1
  • Type changed from defect (bug) to enhancement

This needs some more work... E.g. the reposition function is a mess, just wanted to upload everything so far. :) Seems to work well. We were also thinking about adding a button to add a caption when there is none, so I'll consider adding that too.

This ticket was mentioned in Slack in #feature-imageflow by janneke. View the logs.


9 years ago

#5 @iseulde
9 years ago

Oh, and this is now a TinyMCE toolbar, which means we could allow plugins to add new buttons.

This ticket was mentioned in Slack in #feature-imageflow by azaozz. View the logs.


9 years ago

@azaozz
9 years ago

#7 @azaozz
9 years ago

In 30147.2.patch:

  • Changed the positioning to make it more consistent when editor expand is on/off.
  • Fixed adding/removing of alignnone class and repositioning after that.
  • Added a bit stronger box-shadow and border to make the image toolbar more defined.

IMHO this works and looks quite well, ready to be committed.

@azaozz
9 years ago

Remove unused var.

@azaozz
9 years ago

#8 @azaozz
9 years ago

In 30147.4.patch:

  • Change the toolbar position to be over the image for iOS, so it doesn't clash with the default popup toolbar there.
  • Fix various iOS Safari bugs.
  • Don't hide when the parent window scrolls unless editor-expand is enabled.

@azaozz
9 years ago

@azaozz
9 years ago

@azaozz
9 years ago

@azaozz
9 years ago

This ticket was mentioned in Slack in #core by azaozz. View the logs.


9 years ago

#10 @azaozz
9 years ago

In 30318:

TinyMCE: enhance the inline toolbar for images:

  • Add alignment (left, center, right, none) buttons.
  • Position the menu above the image when possible, except on iOS.
  • Fix selecting images on iOS.

First run, part props avryl. See #30147.

#11 @azaozz
9 years ago

In 30319:

Fix typo in translatable string, see #30147.

#12 @johnbillion
9 years ago

This is pretty neat.

avryl mentioned that it might be possible to keep the toolbar in place under the cursor until the user moves the cursor away. This means if you are swapping an image between left and right alignment then the toolbar doesn't keep moving away. Do you think this might be possible?

#13 @azaozz
9 years ago

Yes, it is possible. However when floating an image, it often moves vertically as well as horizontally. Then the toolbar can end in very awkward places, even off the screen. We can detect where the image has moved and hide the toolbar if it is "too far". That would be somewhat inconsistent behaviour though.

#14 @azaozz
9 years ago

In 30339:

TinyMCE: set the image toolbar's z-index higher than the editor when the new DFW is active. See #30147.

#15 @afercia
9 years ago

Very nice, thanks very much @azaozz, @avryl :)

Noticed a small thing to fix, very edge case, when the toolbar is close to the editor right edge, the "triangle" needs some attention, see its position and "faux" border:

https://cldup.com/Mjv__0AfDN.png

there are different ways to do that, not sure which one to use, proposed patch resets the negative margin and use position, though you might prefer a different approach.

#16 @azaozz
9 years ago

In 30361:

TinyMCE: don't show image toolbar for placeholder images. See #30147.

#17 @azaozz
9 years ago

In 30362:

Fix typo in [30361], see #30147.

#18 @azaozz
9 years ago

In 30363:

TinyMCE: fix the border on the image toolbar arrow. Props afercia, see #30147.

#19 @afercia
9 years ago

For accessibility considerations, please see #27642 which still applies even with this new image toolbar.

#20 @johnbillion
9 years ago

The "Remove alignment" button should be renamed "No alignment", especially as it's toggled by default.

#21 @azaozz
9 years ago

In [30385]:

TinyMCE: fix the tooltip for 'alignnone' button on the image toolbar. Props johnbillion, see #30147.

#22 @johnbillion
9 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Marking this as complete. New issues can go into separate tickets.

#23 @azaozz
9 years ago

In 30509:

TinyMCE: set the image toolbar's z-index to be the same as the other TinyMCE panels, or it is under the editor when in fullscreen mode. See #30147.

This ticket was mentioned in Slack in #design by helen. View the logs.


9 years ago

#25 @azaozz
9 years ago

In 30558:

TinyMCE: use the actual image node when calculating the position of the toolbar. Fixes a bug in old IE. See #30147.

#26 @azaozz
9 years ago

  • Owner set to azaozz

In 30560:

TinyMCE: don't hide the image toolbar when the iframe window fires onresize. There is a bug in several browsers that triggers onresize when a tooltip is shown, only in RTL mode. Also use better variable name. Fixes #30147.

Note: See TracTickets for help on using tickets.