Giter Site home page Giter Site logo

Comments (16)

jalal246 avatar jalal246 commented on May 11, 2024 1

Very interesting case to be honest. I'd love to take a look into the code (drop-down styling) if that's possible. Of course, cleaning element styling and attributes is possible.

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024 1

Oh I didn't mean to not transform at all, I love how you implemented that!

Just after the dragging interaction is finished (with endDragging) you shouldn't leave the translate3d(0,0,0) there. What already happens is that dflex cleans up the other attributes like z-index: 99 and position: relative after one specific drag interaction has ended and the DOM has been mutated.

I am not sure if you know what that does, so this may seem obvious, sorry in this case:
For the average user leaving translate3d(0,0,0) doesn't seem like an issue cause it's not actually translating by any amount. You'll notice however that even if you translate something by 0, it creates a new z-index stacking context. It is a side-effect that dflex can afterwards clean up and only add it back after a user starts dragging something else again (just like dflex is already doing with the other two attributes mentioned in the previous paragraph).

Didn't you know that, or am I just not seeing something?

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024 1

Yes if it's necessary. Because changing the context of the element has a cost. The browser will repaint again if the user decides to do another round of drag and drop. That's why it's preserved even when you reconcile. because it's more friendly to the browser.

I see - You know better than me, if the performance allows updating so many translates per second while actively dragging, does it really have an impact to change the style once for each mousedown/up?

If it does, then this should probably be a flag you can enable if you need it. If it doesn't make an impact really, either change the behavior to always remove on reconcile OR make it a flag but with the default behavior of not leaving those transform: translate3d(0,0,0) traces after reconcilation.

If you do come to a decision, I'd love to try if I can prepare a PR with the necessary changes :)

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

Hi, @xdivby0 thanks for opening this issue.

  • For the dragged element: The current behavior is cleaning up what's connected directly to the dragging process. If DFlex is adding z-index: 99 for the sake of dragging over other elements then it should be removed after the process ends.

  • For the affected elements: If the element is registered then it's expected to be transformed. So the default is translate3d(0px, 0px, 0px). Because you can always transform without composing the DOM. This means you can indefinitely transform an element inside its container or to another container without changing the DOM tree. This approach is meant to enhance drag-and-drop performance, making it insanely fast. You can try it by setting changing commit default value and setting it to false:

commit: {
  enableAfterEndingDrag: false,
},

When removing translate3d?

If the element is reconciled to the DOM tree. Then its state is empty. No transformation yet.

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024

Thanks for the quick response!

For me the DOM is updated with the changed order after I let go of the dragged element at the new position. Still the translate3d(0,0,0) is there, which messes up stacking context.

From my point of view, after I let go of the element and I have the new order of the elements via the mutation listener, I can remove the translate3d, at least it works for my use case. It automatically gets added by dflex after I try to move anything again (which still works and seems to be efficient). Wouldn't it make sense to just remove the transform property instead of just setting it to translate3d(0,0,0)?

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

For me the DOM is updated with the changed order after I let go of the dragged element at the new position. Still the translate3d(0,0,0) is there, which messes up stacking context.

If the element isn't reconciled then it stays as it is. For example: if before releasing the drag element with my-id is positioned correctly then it stays having its transformation data This element has been transformed and then went back to its origin. DFlex ignores elements that have the right position and minimizes interaction with DOM as possible.

Wouldn't it make sense to just remove the transform property instead of just setting it to translate3d(0,0,0)?

Yes if it's repositioned.

Think of it as a transformation state:

  1. Empty: The element hasn't transformed at all.

  2. Reset: The element traveled and went back to its origin. (your case)

  3. Stateful: The element transformed and waiting for DOM mutation.

It could be a bug if:

A. The element has translate3d(0,0,0) even if it didn't move its position at all. This is not the expected behavior.

B. If it's causing issues with your app for some reason that I am not aware of.

otherwise, it could be annoying but it's okay since the point is transformation after all.

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024

I am not sure if I understand 100% but here's 4 draggable elements before dragging them (the 4 columns are wrapped into the draggable Component I just copied from your example page):
image

Now as you can see if I open the dropdown from the second element, it displays correctly. When I drag the second element just a tiny bit and let it snap back to its original place (or just change it's place with the third element for example), this is the result:

image
This caused by the one or several of the elements now having the transform: translate3d(0,0,0) value. If I go in via the developer tools and remove the style, it's working correctly again.

If nothing speaks against it, couldn't dflex just remove the transform property after being finished (no matter if the element snapped back to its origin or new place)? It would certainly have fewer side-effects like creating new stacking context or maybe other issues

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024

I think it's a bit niche case, yes. Still, why deal more side-effects than necessary :) If you think I could try preparing a PR, tell me, I'll gladly do that.

For the code of the dropdown: I've made a typed component for the dropdown, actually it's a searchable dropdown (just super-basic for now). It's highly specialized though because it's made for a specific form-library (remix-validated-form) for the Remix framework (which is in turn based on React).

So here's the probably relevant part of the component for styling and html structure. It uses tailwindcss in case you don't recognize the class names:

    <div className="relative">
      <label className="text-primary-800/70" htmlFor={props.name}>{props.label}</label>
      // hidden input is needed to actually select the id, not the display value
      <input type="hidden" name={props.name} value={value}
             {...getInputProps({ id: props.name })}
      />

      // here comes the actual visible text container in which you type in something for searching
      <div className={`relative`}>
        <input
          onChange={(e) => {
            setVisibleElements([...props.data].filter(x => props.searchFilter(x, e.target.value)));
            setText(e.target.value)
          }}
          value={text}
          onFocus={() => setFocus(true)}
          onBlur={() => setTimeout(()=>setFocus(false), 100)}
          className={`w-full block bg-white font-semibold px-2 py-1 rounded mt-1 outline-none
      ${error ? "border-red-400" : "border-transparent focus:border-primary-500"} border-2 `}
        />

        // the part that actually drops down and gets problematic when the stacking context is manipulated
        {focus &&
          <div className="z-10 translate-x-1 max-h-48 overflow-y-auto mt-2 absolute bg-white p-1.5 shadow-md rounded-md">
            {visibleElements.map(element =>
              <div className="px-4 py-2 rounded cursor-pointer hover:bg-primary-100" onClick={()=>{
                setValue(element[props.selectAttribute]);
                setText(element[props.renderAttribute])
              }}>
                {
                  element[props.renderAttribute]  // renderAttribute is "name" in this case, while selectAttribute is "id". Both are possible fields of the elements displayed
                }
              </div>)}
          </div>
        }
      </div>

      // rest is error/hints
      {props.helperText &&
        <p className={`text-primary-800/60 text-sm`}>{props.helperText}</p>
      }
      {error &&
        <span className="text-red-500">{error}</span>
      }
    </div>

If you have any questions or are interested in the whole component for actually using it somewhere, I can give that to you too, just didn't paste it here because not relevant for the issue :)

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

why deal more side-effects than necessary

It's the opposite. The rule is to minimize the DOM interaction that's why transformation is there. Less is more performative.

If there is a repo or codesandbox where I can try it live would be easier to deal with it. For the contribution, I'd love to. Maybe adding a new case to the playground to test it and deal with it.

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

I got your point but for me, I am not aware of the issue that's creating. As the DFlex element is expected to be transformed. The assumption is that all elements should be expected to have their transformation whether it's zero or not.

This is a normal scenario:

image

So the ground zero is translate3d(0,0,0). I am digging into the stacking context. Thanks for the reference. Still, is it a better approach to preserve stacking context for each element or just create/remove a new one with every interaction? We could make it customized depending on the case where some cases require a total clean-up.

What I usually do is create a route in the playground to deal with different cases and then develop and test the solution Cypress/Playwright. That's why playground has some unsolved issues #597

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024

Making it a flag when registering the element in the store or some similar customization would defo work for me, I am just not sure if it would mean unnecessary complexity in dflex's code + for devs using it since I can't imagine a use case where it would be beneficial to not remove the transform translate property and add it with every new interaction again.

I am not sure if I find the time to set up a playground example to demonstrate, but I'll try to do today or tomorrow.

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024

This is a normal scenario:

image

Wait, is this only while you are still holding down the mouse button? I have similar transforms while I am dragging / holding the mouse down. But once I let go, everything is reconciled with the DOM once and then the transform translate values are all 0. Is that weird or normal?

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

Nope. This is after releasing the drag. There's an option where you can control the reconciliation. You can set it to false and trigger store.commit() when needed.

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

Elements' positions and transformations are preserved internally. It's a hard approach to implement but it's also fast. Even changing the dragged position is conditional depending on each scenario. For example, the default is not to clone and ghost unless it's necessary.

from dflex.

xdivby0 avatar xdivby0 commented on May 11, 2024

Okay, I didn't touch that option, so the default is to always reconcile after a drag has finished, correct?

In that case, shouldn't the default also be to clean up the translate property after reconciling (because of course if the elements still have values inside the translate property, we can't just remove it). Maybe we can make removing the translate property part of the reconcilation?

from dflex.

jalal246 avatar jalal246 commented on May 11, 2024

Okay, I didn't touch that option, so the default is to always reconcile after a drag has finished, correct?

Yes.

Maybe we can make removing the translate property part of the reconcilation?

Yes if it's necessary. Because changing the context of the element has a cost. The browser will repaint again if the user decides to do another round of drag and drop. That's why it's preserved even when you reconcile. because it's more friendly to the browser.

from dflex.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.