Managing z-index/node positions on ds-selector areas

owenfar commented 2 years ago

Describe the bug In v2, all ds-selector areas have been moved into the main body node, and we're keeping states for multiple specific dragSelect areas to hide & show the right one accordingly. The problem now is managing z-index. Since all ds-selector areas are under one element at the top of the node tree, it's impossible to keep a "window" above another dragSelect area, as we do not have control of the z-index anymore. One example would be a "window" above a "desktop".. the desktop ds-selector would go above the whole window as it is now:

154266531-ee847b34-97d9-46ee-af3e-9ca1bd2829e2

To Reproduce Create multiple dragSelect areas were one overlaps the other.

See live example: https://codepen.io/owenfar/pen/ZEooKOX

Expected behavior The dragSelect should be positioned in the areas themselves, this way the z-index is taken care of automatically. I know that this was part of an update to allow/fix/enhance a feature such as the one described in #63. So I know that it's a complicated aspect and a big part of the library itself.

There are some things I imagine would be the perfect scenario for the ds-selector:

That we have only one ds-selector globally, and it is moved according to the current clicked position if a ds-area exists - I know this must create other complexities, it's more for the sake of less-is-better.
Re-accomplish issue #63 by determining the max-height, max-width of the current area and stop the scrolling process once the max scroll position is reached - My best guess regarding how we would tackle the selector going outside the window would be to follow the cursor on the whole viewport, instead of just the specific area? (setting overflow:hidden state can be applied to the parent element by default or left up to the user).

Thank you!

ThibaultJanBeyer commented 2 years ago

Hi @owenfar and thanks for your patience and the elaborated description, I might have some time in the following weeks to look at this. But I am not sure how to tackle it right out of my head. Because as you mentioned it was designed like this so that one can drag outside of the window as in #63.

owenfar commented 2 years ago

Hello @ThibaultJanBeyer, it's nice to hear back from you. By coincidence, I just started last week dedicating time on my project again.

Brief overview

I can only give a little guidance perhaps. I still think that the second option I mentioned above is the best direction forward - if possible. We have three priorities:

Stop scrolling when reaching the bottom section (original problem - fixed with v2)
Have the ds-selector element within the scope of the ds-area element (new problem)
Possibly support cursor outside "windows" (added feature - v2)

The new update works really well, but unfortunately when we have multiple ds areas, we lose control over z-index - this is most ideal for desktop UI's, so perhaps a niche (but it could apply to many other views were dragSelect lives under a "popup").

Moving forward

To reiterate, let's say we have the ds-selector back inside the ds-area, so that we have control over it's z-position within different scopes/windows/etc. Talking about our first priority, would it be possible to stop scrolling by determining the max-height of the current scrollable area once the max-scroll position is reached? I know it is possible to get the "scrollable" height of a scrollable element, so technically I imagine it can be solved by a condition. This in turn would solve problem 2.

For number 3, the only method I have on top of my head now is that the ds-area is wrapped around a parent element that has overflow hidden. Then ds-selector can keep on stretching outside the ds-area without affecting scrolling or other views. Basically it will always follow the cursor, whether or not it is in a "window" (or a ds-area that is smaller than the viewport width and/or height).

...

I hope I was clear enough, it can be hard to imagine what I have in mind sometimes - so please do let me know if something is not clear, and I will explain with the help of images perhaps.

Thank you so much. Looking forward to hearing back from you.

ThibaultJanBeyer commented 2 years ago

Hey @owenfar thanks for your patience 🤗 I think this could also be solved if one is able to decide where the selector should be placed, i.e. being able to select the selector areas parent element, that might be easier. Not sure how well this will work out tho’ because the whole reason why I moved the selector out of the area was that it lead to a lot of other issues within the are (since the area can potentially be anything) so I think ideally it would be outside. Did not have time yet to check it, have been fixing another issue (the one with updating any settings after initialization) but this one is next on my list =)

PS: you do have control over the z-index it’s on a new element with the default class name ds-selector-area and has the maximum possible z-index by default. So maybe just changing the elements z-index to something lower solves your issue?

owenfar commented 2 years ago

Hey @ThibaultJanBeyer, great to hear that :) Very happy to see you dedicating time on this library again.

I had already tried to change the z-index before, the problem then was that all ds-selector elements were placed at the top most node. This by default gave each selector the highest priority unless the "window" elements are also at the same node level. Even so, it became too complicated to organize multiple windows with different z-index based on priority, whereas I see these issues would automatically be solved by keeping the selector within the select-area (as we had in v1) or at least in parent area. But you know more regarding the other issues that might arise from this.

If you feel that this is a niche or that it's not intended for my use-case, I will completely understand this. I think I'm only bugging about this because it was like that in v1 and worked very well for me :D

Are there more issues you fixed from moving the ds-selector out than what was targeted in #63 ? If not, do you think my suggestions for point 1 & 3 could work (previous comment)?

Thank you for your time.

ThibaultJanBeyer commented 2 years ago

Yes, there were definitively other issues with it. But do you think adding an option to the tool where you can choose where to place the selector area, i.e. you could choose that to be the area itself then it would be back inside of the area 🤔

owenfar commented 2 years ago

Yes we can do that definitely. But won't this then still require to fix the issues we had when having it within the area itself, if we choose this option?

ThibaultJanBeyer commented 2 years ago

Woah speedy answer 🚤 💟

Yes we can do that definitely. But won't this then still require fixing the issues we had when having it within the area itself, if we choose this option?

You're right it might not work for every use-case because we don’t know where the user places it in the end. That was why I took it out, this gives control over it and works for 90% of the use cases. I still agree tho’ that the user should be able to place elements on top of it 🤔

I think we’ll have to play around a bit with different solutions before finding the optimal one, this is definitively not straightforward.

Given the option to choose where to place it gives also the responsibility to the user to test whether that placement even works. And removes that responsibility from the library.

I would also wanna try whether just making the z-index customizable couldn’t solve it also. Or maybe it’s a combination of both 🤔

owenfar commented 2 years ago

I agree with all your points.

Regarding the z-index, that will only work if the elements live in the same node level though - this would still fail if you have a ds-area within an ds-area (e.g. desktop and child window elements). The parent will always win right, no matter the z-index value.

I would love to have the option to have it back within the area, this would allow the niche use-cases (like me :)) to still be able to update the library to the latest.

I'de really like to work to solve the issues that we had before when it was within the area. I do believe it's very much possible, and would also like to try out myself.

owenfar commented 2 years ago

@ThibaultJanBeyer actually now that I think about it, perhaps having the option to place it within the area could already be enough :) We should try this option first

ThibaultJanBeyer commented 2 years ago

Cool yeah, that hopefully is not too complex. Do you mind sharing a code example on some code-sharing website that I can work with while testing? That would be super helpful :)

owenfar commented 2 years ago

Yes I will do that a bit later today, or by tomorrow for sure if I don't get a chance :)

owenfar commented 2 years ago

Hello @ThibaultJanBeyer,

As promised please find a live playground for such a scenario: https://codepen.io/owenfar/pen/ZEooKOX

As you can see, here we have a simple but fairly similar UI implementation with a "window" ds-area on top of a "parent" ds-area. The issues are immediately present, but I wanted to point them out here for clarity:

When dragging from "parent" element, it is going over the window element + you can also drag the window element as can be seen in the video attached below:

https://user-images.githubusercontent.com/25148235/195708109-f106d56b-2eff-401a-a3b9-5ed5e1c720ca.mov

One more minor thing I want to point out is that break() in predragstart still somehow creates a tiny square from the parent ds-selector but this would be automatically hidden if z-index was working correctly. Again, a very minor bug though:

Let me know your thoughts now that everything is more clear :) Thank you so much

ThibaultJanBeyer commented 2 years ago

All right, thanks a lot!

ThibaultJanBeyer commented 2 years ago

Hey @owenfar, I started working on this and given your example, theoretically just changing the z-index on the selector areas and the container would work, see this:

https://user-images.githubusercontent.com/12467511/200790698-c3dff940-1864-457e-ad84-d92a473fb8ac.mp4

I only changed the CSS to this:

body { margin: 0; }

::selection { background: none !important; }
::-moz-selection { background: none !important; }

.ds-area {
  width: 100%;
  min-height: 100vh;
}

.selectable {
  width: 50px;
  height: 50px;
  margin: 10px;
  background: #ccc;
  display: inline-block;
}

  .selected {
    background: black;
  }

.w-ds-area {
  position: absolute;
  top: 160px;
  left: 200px;
  width: 420px;
  height: 240px;
  border: 1px solid black;
  overflow-y: scroll;
  background: white;
  z-index: 10
}

.ds-selector-area:nth-child(1) {
  z-index: 9 !important
}

.ds-selector-area:nth-child(2) {
  z-index: 11 !important
}

As you can see, I added the .ds-selector-area:nth-child(x) to override the default z-index of the selector area.
And added a z-index as well as a background color to the .w-ds-area.

I do think it makes sense to give the ability to set the z-index for the ds-selector-areas via the settings, then there is no need for !important and :nth-child(x). BUT do you really think it still needs an option to chose where to place the selector area in the DOM? If yes, why and can you share an example highlighting this?

If you agree that the z-index is enough or not, either way, would you want to create that PR? This would be a very easy addition to get familiar with the codebase and add you to the list of official contributors, which would be well deserved in my opinion =)

Thank you! 🤗

owenfar commented 2 years ago

Hello @ThibaultJanBeyer, thank you for reaching back about this :)

I applied the changes you suggested and saved them: https://codepen.io/owenfar/pen/ZEooKOX but it still doesn't work for me 🤔 strange. I am using latest Chrome on macOS.. perhaps it's browser related?

Theoretically this shouldn't work though, since ds-selector-area is in the same node level as <main> node, so all child elements of 'main' technically cannot adjust based on z-index coming from ds-selector-area ... but I don't know now maybe I'm missing something important here..

owenfar commented 2 years ago

Here's a video showing the latest update in incognito mode:

https://user-images.githubusercontent.com/25148235/200796930-cda5d9ba-17b9-4d68-893d-4c58542501ac.mov

ThibaultJanBeyer commented 2 years ago

Yes true sorry the nth-child selector was wrong, my mistake. In CSS the correct would be nth-last-child(x) since they are the last elements in the DOM. So basically:

Replace the :nth-child ones with:

.ds-selector-area:nth-last-child(1) {
  z-index: 9 !important
}

.ds-selector-area:nth-last-child(2) {
  z-index: 11 !important
}

And it will work!

Of course this is a hacky solution, I just made it as an example to show you that it would work if one could set the z-index on the area.

-- Just for reference, here the same but with the dev tools:

https://user-images.githubusercontent.com/12467511/200809160-be68be80-d5a9-455b-9ae6-70a70b4e63a1.mp4

If you’re not ready to contribute via code that’s ok I can do it then myself too, I was thinking that it would be cool to have you as code contributor :) but that’s up to you.

I was thinking of adding a setting like this:

new DragSelect({
        …
        selectorZindex: 1
        …
});

To set the z-index of that DragSelect selector area instance. What do you think?

owenfar commented 2 years ago

Hello @ThibaultJanBeyer, I did the update now and it did work like this 🤔 - but this only does work if we assume none of the other elements in the same node level have z-index set, otherwise my explanation still stands true. Doing this will also trigger a bug when from the first click the z-index is not yet adjusted (thus it still goes over the "child" DS area), until the browser detects there are non set - which also proves how z-index works in a node-level specificity.

Here's a video that will clearly show everything I mean:

https://user-images.githubusercontent.com/25148235/200815358-431ac384-f926-48b4-9ae1-e315921518e3.mp4

I hope you understand that this to me still feels like a hacky solution - technically managing z-index this way will not always work as intended. There is also the case where the parent drag-select still scrolls the child DS area, but this could be another bug separate from this one.

What do you think then 😞? I don't want to be a pain, I'm only trying to clearly explain that it's not the most ideal way to do this perhaps.

I do understand such code-bases and would love to dedicate time, so if you think this is not a priority, we can leave it as is, and I can eventually do a PR and try to implement something else, perhaps to choose where the ds-selector is placed?

In any case, I appreciate you dedicating time for this.

ThibaultJanBeyer commented 2 years ago

Oh yes sorry, I checked your profile you are pretty skilled indeed. So what you write makes sense, I did actually not know that the z-index is not applied right from the start only after an interaction with the page happened TIL :O Do you have by any chance a link to some docs/article that explains why that is? I’d like to understand this in more detail :)

It’s up to you, I have some slack time this week and would be able to implement a setting to choose where to place the ds-selector-area, I would still also add the z-index modification option even if you’re right, that it does not always solve all use-cases I think it’s still a nice to have. If you have no time yourself then no worries, you’ll certainly get the opportunity at some point, you’re already helping a lot by answering other issues btw, thanks a lot for that! :)

So I am thinking about something like this:

new DragSelect({
        …
        selectorAreaIndex: 1, // default: max z-index
        selectorAreaParent: <node /> // default: <body />
        …
})

wdyt? Cheers 🍻

owenfar commented 2 years ago

Hello @ThibaultJanBeyer, I am no expert by no means though :D Regarding z-index, what I learned from experience is that z-index will only have meaning within it's parent element.. so outside the parent it will not work as expected unless nodes are in the same level. Now regarding the detection of z-index within the node tree I think it has to do with the stacking context.. but again, I don't know much just that I know it exists :) here's the technical details about how it works.

I like the idea of how you're going with implementing this (index and area parent), and if you have the time you'll definitely get it done more quickly than me hehe :) I am working on external projects and currently quite booked with work - but as I promised like years ago, I would eventually get the time, which I'm slowly heading towards.

Thank you so much for understanding and taking all this time to read my suggestions and comments

ThibaultJanBeyer commented 1 year ago

Hi @owenfar I finally had time to look into this. Actually while building it, it became pretty clear that it would be better to just give an option to pass your own custom selector area (just like you can use your own selector element).

The API design is just:

new DragSelect({
        …
        selectorArea: <node />
})

Please have a look at the PR, try it out and let me know what you think. Thank you =)

owenfar commented 1 year ago

Hello @ThibaultJanBeyer, this is really great :) Thank you so much for the time and effort. Will try it out over the weekend and reach back here.

owenfar commented 1 year ago

Hello @ThibaultJanBeyer, I have tested this over the weekend, and it works fine for simple scenarios.

Did you notice that cursor and selector are misaligned when selectorArea is not set to viewport width & height?

As I could also see in the testing file selector-area-overlap.html you left out the child window from having a specific selectorArea, and perhaps this was intentional because you also know that it doesn't work well when the parent element is not set as full viewport width & height.

Just to be clear, selectorArea: will only work on areas that are fullscreen relative to the viewport, once that view is adjusted, the selector is misaligned from the cursor position.

I'm not sure it's ideal to have it as part of the main library as it is, because it's clear that it breaks in certain use-cases.

I am sorry for not dedicating time from my end. I am very much interested, but I'm waiting for the right time to be 100% focused on implementing such a feature.

If you want, we can close this issue for now, I don't want to keep dragging this along and take more of your time for what seems only to be important to me at the moment.

Many thanks in advance :)

ThibaultJanBeyer commented 1 year ago

So what you are saying is that this solution does not solve your use-case? I think it might be worth looking into it, no? Like this uncovered some other issues like how nesting of areas result of triggers in other areas and auto-scroll of other areas. If it’s not really important for you right now, I’ll pause it and look at the other open issues and come back to this one later. But would really appreciate your contribution here :)

owenfar commented 1 year ago

Yes exactly - I think all the problems come from having nested drag-select areas. Perhaps not a popular request at the moment :) I would leave this as an improvement later on yes, and definitely you can focus on other issues. Thank you so much for your time.

ThibaultJanBeyer / DragSelect

Managing z-index/node positions on ds-selector areas #125