Problem/Motivation

We identified a couple of issues and regressions regarding the drag & drop handles, some related to #3112668 and some not.

  • The handles are vertically misaligned
  • The handles horizontal width was increased and they take more horizontal space than needed
  • The change (*) indicator is misaligned and looks broken

Proposed resolution

Fix the issues identified above.

CommentFileSizeAuthor
#11 Selection_094.png54.34 KBpivica
#11 Selection_093.png54.56 KBpivica
#11 Selection_092.png49.71 KBpivica
#11 Selection_091.png49.38 KBpivica
#10 Patch #10 - claro.png68.12 KBsasanikolic
#10 Patch #10 - seven.png44.59 KBsasanikolic
#10 3125662-drag-drop-handles-improvements-10.patch2.11 KBsasanikolic
#10 3125662-drag-drop-handles-improvements-5-10-interdiff.txt1.15 KBsasanikolic
#6 3125662-drag-drop-handles-improvements-6.patch2.27 KBpivica
#6 3125662-drag-drop-handles-improvements-6-interdiff.txt1.28 KBpivica
#6 Selection_047.png123.81 KBpivica
#5 3125662-drag-drop-handles-improvements-5.patch2.11 KBsasanikolic
#5 3125662-drag-drop-handles-improvements-5-interdiff.txt714 bytessasanikolic
#5 Alignment improvement.png179.24 KBsasanikolic
#3 3125662-drag-drop-handles-improvements.patch2.02 KBsasanikolic
#3 Seven theme paragraphs handles.png103.61 KBsasanikolic
#3 Claro paragraphs handles.png109.88 KBsasanikolic
drag&drop handle bar issues.png86.6 KBsasanikolic

Comments

sasanikolic created an issue. See original summary.

sasanikolic’s picture

sasanikolic commented
Issue summary: View changes
sasanikolic’s picture

sasanikolic commented
Status: Active » Needs review
StatusFileSize
new Claro paragraphs handles.png109.88 KB
new Seven theme paragraphs handles.png103.61 KB
new 3125662-drag-drop-handles-improvements.patch2.02 KB

Here is the patch with the proposed improvements:

  • Aligned the handles to the left, so it's in-line with the PARAGRAPHS label above.
  • Improved vertical position of the handle
  • Improved the horizontal space
  • Aligned the * when a paragraph is moved
  • EXTRA: Added same hover effect as in Claro - not sure we want to do that in order to be consistent with the current theme or can we alter this behavior?

Attaching the patch with the screenshots of Claro ad Seven theme.

sasanikolic’s picture

sasanikolic commented

Note that I tried to replicate the solution of Claro for the * display (which increases the horizontal space when * is added), but could not solve it with flex or display:table, as then there was no way to vertically center the handle on expanded text/image paragraphs. Claro has manipulated the markup with an extra div wrapper for that solution.

sasanikolic’s picture

sasanikolic commented
StatusFileSize
new Alignment improvement.png179.24 KB
new 3125662-drag-drop-handles-improvements-5-interdiff.txt714 bytes
new 3125662-drag-drop-handles-improvements-5.patch2.11 KB

Here is another small fix for the alignment improvement.

pivica’s picture

pivica commented
Issue summary: View changes
StatusFileSize
new Selection_047.png123.81 KB
new 3125662-drag-drop-handles-improvements-6-interdiff.txt1.28 KB
new 3125662-drag-drop-handles-improvements-6.patch2.27 KB

Checked the code changes and tested visually, looks good.

> Note that I tried to replicate the solution of Claro for the * display

Not sure what do you mean, but i guess we could centre * a bit more, tried that and it looks nicer:

What do you think @sasanikolic?

  • Berdir committed 8d0e4ce on 8.x-1.x authored by pivica 
    Issue #3125662 by sasanikolic, pivica: Drag & drop handles improvements

  • Berdir committed 148e29b on 8.x-1.x 
    Revert "Issue #3125662 by sasanikolic, pivica: Drag...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

I had that already committed but apparently the * is then over the paragraph type in claro with that.

sasanikolic’s picture

sasanikolic commented
Status: Needs work » Needs review
StatusFileSize
new 3125662-drag-drop-handles-improvements-5-10-interdiff.txt1.15 KB
new 3125662-drag-drop-handles-improvements-10.patch2.11 KB
new Patch #10 - seven.png44.59 KB
new Patch #10 - claro.png68.12 KB

I'd also like to fix this with flex, but i noticed that this definition in the patch #6 breaks too many things, the arrows are not centered in claro and the arrows overlap with the content (in claro, core issue where 3 arrows are displayed - not sure what's the status with that issue).

I adapted the styles for some better alignment and the focus area centering for Claro in this patch.

Can you test again @pivica?

pivica’s picture

pivica commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Selection_091.png49.38 KB
new Selection_092.png49.71 KB
new Selection_093.png54.56 KB
new Selection_094.png54.34 KB

Tested against Seven and Claro in 8.9.2.

Noticed that '*' sign is not centered any more in Seven but it is also not centered with patch from #6, probably something else changed somewhere else so that is fine.

Claro looks better with latest patch from #10. The only problem is that Claro is defining width: 1.5rem for '*' sign which is moving the whole column a bit more to the right:

This is something that Claro do so i guess they have a reason for this. Maybe we want to consider later to make this 1.5rem smaller because we do need horizontal space as much as possible especially for the small screens:

With Claro `min-width: 1.5rem` on `.tabledrag-cell--only-drag .tabledrag-changed` element (notice we are overflowing to the right):

Without 1.5rem min-width:

But i guess we should check all this when we switch to D9, because Claro in 8.9.2 is still marked as experimental - maybe they fixed all this things in D9?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.