backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

Tabledrag hierarchy is broken in Basis #2689

Open docwilmot opened 7 years ago

docwilmot commented 7 years ago

Steps to reproduce

docwilmot commented 7 years ago

I've tracked this down to one line in Basis css/base.css: line 6:

* {
  box-sizing: border-box;
}

Commenting this out enables lateral-movement tabledrag again. But even though the rows move laterally, the tabledrag handle stays fixed to the left table edge because Basis also has a position: absolute; on the handle CSS in components/tabledrag.css line 56.

tabledrag

So in summary, two things to fix this: the box-sizing, and the drag-handle positioning. I am not sure what those fixes should be. Tagging @wesruv.

serundeputy commented 7 years ago

I agree with your conclusions here @docwilmot My CSS foo is not strong enough for this.

@wesruv is there someway we can scope those kinds of rules from base.css and tabledrag.css to only apply if the user is on the front end i.e. if the url is of the form /admin/% then these rules should not fire?

indigoxela commented 3 years ago

What would be the desired result here, what should this look like? Proper indentation, for sure, but what else? A quick mockup could help.

indigoxela commented 3 years ago

Here's a PR as a starting point for further discussion. Currently it only fixes indentation.

I expect this issue to be a tricky one, as fixing the hierarchy handling might need deeper changes - potentially problematic ones regarding existing subthemes out there.

A screenshot of what we have so far: term-hierarchy-basis

docwilmot commented 3 years ago

Seen; I made a term list at http://3572.backdrop.backdrop.qa.backdropcms.org/admin/structure/taxonomy/draggable for testing.

I expect this issue to be a tricky one ... regarding existing subthemes out there.

If it never worked, I think we should fix it if we can.

klonos commented 3 years ago

I expect this issue to be a tricky one...

Yeah. It seems that the current design did not take nested table rows into consideration. The drag handler should be immediately next to each item. Here's how things look with the Seven admin theme in Backdrop for reference:

image

...and in D8 with Claro:

image

indigoxela commented 3 years ago

It seems that the current design did not take nested table rows into consideration. The drag handler should be immediately next to each item.

I agree, this is the main problem here. Moving the drag handler next to the items contradicts the design concept and might look awkward. ... And needs a lot of rewrite.

indigoxela commented 3 years ago

Before we re-invent the wheel... maybe we can circumvent the problem of a bigger design change with more significant indent indication?

I pushed an experimental (and by no means elaborated!) approach to my PR. Not sure how good the UX is.

A screenshot with a hierarchy: term-hierarchy-basis-2

wesruv commented 3 years ago

I can take a look this week

wesruv commented 3 years ago

Made a PR https://github.com/backdrop/backdrop/pull/3578

The CSS for tabledrag is gnarly, it could be so much simpler if the indentation divs were added before the tabledrag link, but that's not how it works 😭

I simplified the look, which is a little sad, but we'd need much better JS/DOM order for me to have fancy styles like I did.

image

indigoxela commented 3 years ago

@wesruv Thank you so much for digging into this. :+1:

Two minor issues:

  1. The tree indication when moving a part of the hierarchy seems broken (the lines that indicate parent/child relations don't fully show up when moving a part of the tree (LTR))
  2. The RTL hierarchy isn't showing up yet, they're aligned right without indentation

Otherwise it's looking good. Yes, it's a bit sad that the fancy style had to be dropped in favor of proper hierarchy display...

klonos commented 3 years ago

it's a bit sad that the fancy style had to be dropped in favor of proper hierarchy display

Sad, but necessary. In the previous design we seem to have overlooked the use case of hierarchical tabledrag 🤷🏼

indigoxela commented 2 months ago

After years without additional feedback and because CSS changes are known to be hard, I'm closing my PR. No need for another PR queue zombie. :laughing: