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

Sequenced selections are buggy #196

Closed
adelin-b opened this issue Apr 4, 2023 · 5 comments
Closed

Sequenced selections are buggy #196

adelin-b opened this issue Apr 4, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@adelin-b
Copy link

adelin-b commented Apr 4, 2023

What is the problem?

Selections in secquences have buggy behavior

What is the current behavior?

Selection doesn't behave logically

Please provide the steps to reproduce and create a CodeSandbox.

To reproduce: https://simonwep.github.io/selection/

S-click + S-click + S-click + ... + S-click

shiftright
Expected: Should add to the selection without removing the first element.

S-click + S-click (This time going up)

leftshiftclick
Expected: It shouldn't deselect all but two files. should select only one like in file manager. (starting and ending element are the same, so only one selected)

click + S-click + C-click + S-click

ctrlshift
Expected: S-click after C-click should select from the C-clicked element and not the first of the whole selection.

What is the expected behavior?

cf expected

Your environment:

Version (@viselect/lastest):
Browser: Chrome 
OS:  linux
@adelin-b adelin-b added the unconfirmed Problem is not confirmed yet label Apr 4, 2023
@simonwep simonwep added bug Something isn't working and removed unconfirmed Problem is not confirmed yet labels Apr 5, 2023
@simonwep simonwep changed the title List of bugs for selections in sequences Sequenced selections are buggy Apr 5, 2023
@simonwep
Copy link
Owner

simonwep commented Apr 5, 2023

Thank you for collecting all these bugs! I'll investigate and fix them this weekend :) Makes me consider rewriting the core of it, it's been almost 5 years haha

@simonwep
Copy link
Owner

simonwep commented Apr 8, 2023

Just a small update, I saw your PR and will review it next weekend (I'm on vacation right now), I'll probably force-push to your branch since you re-formatted the whole codebase, or I'll fix it directly on master and add prettier or so to avoid such issues in the future! Either way, thank you for your time so far!

@adelin-b
Copy link
Author

adelin-b commented Apr 9, 2023

ah shit, yeah I use prettier by default on everyfile I touch, And I do not see how to revert the changes as they are all included.
I recommend using it to avoid having diff poluted with spaces.

@simonwep
Copy link
Owner

The reason I didn't set up prettier for this project in particular so far, is that the code is, from the way it's written, pretty complex and manual formatting helped me maintaining it. Prettier will make it consistent, but at the cost of readability, especially in a non-project environment where you have "little" code, but high complexity :/

@simonwep
Copy link
Owner

Okay, I decided to check out your commit and fix in on master! Thank you for your help, there were quite a few issues with range selection - I decided to, for example, remove range-selection after selecting an "area" entirely because then the "latest element" is subjective to the user. I'll release a patch release shortly after this comment :) Happy holidays!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants