Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve dropzones #534

Merged
merged 9 commits into from
Feb 11, 2022
Merged

improve dropzones #534

merged 9 commits into from
Feb 11, 2022

Conversation

sgratzl
Copy link
Member

@sgratzl sgratzl commented Dec 28, 2021

closes #504

prerequisites:

  • branch is up-to-date with the branch to be merged with, i.e. develop
  • build is successful
  • code is cleaned up and formatted

Summary

move handle

@sgratzl sgratzl added the type: refactor Refactor the current implementation label Dec 28, 2021
@sgratzl sgratzl requested a review from thinkh December 28, 2021 15:19
@sgratzl sgratzl self-assigned this Dec 28, 2021
@netlify
Copy link

netlify bot commented Dec 28, 2021

✔️ Deploy Preview for lineupjs ready!

🔨 Explore the source changes: 69f9b63

🔍 Inspect the deploy log: https://app.netlify.com/sites/lineupjs/deploys/620519348a27880008779c62

😎 Browse the preview: https://deploy-preview-534--lineupjs.netlify.app

@thinkh thinkh requested a review from alexsb December 29, 2021 22:54
@alexsb
Copy link
Contributor

alexsb commented Jan 4, 2022

Awesome, thanks!

Two comments though:

  1. The "place" indicator is as wide as it's drop zone (1/3) in one column, but should probably extend to the next column over as well, as it really doesn't matter whether I'm on the right 1/3 of column A or the left 1/3 of column B.
  2. Once a "sum" indicator appears, it's not possible to get the "place" indicator back for that column, unless you move off the column. We could still have the three drop zones aktive, even if the "sum" indicator is visible, to address this. See the problem here:
    https://user-images.githubusercontent.com/2039375/148050998-29a508db-85c5-4653-a734-fc51af783186.mov

@sgratzl
Copy link
Member Author

sgratzl commented Jan 10, 2022

current version:

dropper

@alexsb
Copy link
Contributor

alexsb commented Jan 13, 2022

This is awesome! Works really nice.

One more suggestion: it might be useful to render the target action (place here, sum, etc.) also (or only?) in the header that you're dragging. You can easily get into situations where the label isn't readable otherwise:
PXL_20220113_115946149

@sgratzl
Copy link
Member Author

sgratzl commented Jan 13, 2022

the header image is rendered by the browser not by me, thus not sure whether I can manipulate it on the fly. Even if it would be to manipulate the original header of the column being dragged within the LineUp header row. Thus, it might be more confusing that helpful.

@alexsb
Copy link
Contributor

alexsb commented Jan 13, 2022

OK, I don't think it's critical, so good to merge as far as I'm concerned.

@puehringer
Copy link
Contributor

One thing I noticed: when moving a column to the right next dropzone, the column of the right neighbor is rendered above the zone.
image

@sgratzl sgratzl linked an issue Jan 21, 2022 that may be closed by this pull request
@puehringer
Copy link
Contributor

One thing I noticed: when moving a column to the right next dropzone, the column of the right neighbor is rendered above the zone. image

Any update on this issue?

@sgratzl
Copy link
Member Author

sgratzl commented Feb 10, 2022

One thing I noticed: when moving a column to the right next dropzone, the column of the right neighbor is rendered above the zone.

image

Copy link
Contributor

@puehringer puehringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me now as expected, I think this PR can be merged as of now 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: refactor Refactor the current implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase size of drop zone for column reordering
3 participants