search

d3lm / ngx-drag-to-select

A lightweight, fast, configurable and reactive drag-to-select component for Angular 10 and beyond
https://www.npmjs.com/package/ngx-drag-to-select
MIT License
293 stars 65 forks source link

feat(lib): add input to control select on click #129

Closed sam-eah closed 3 years ago

sam-eah commented 3 years ago

This small PR gives the ability to chose wether an item should be selected by clicking on it. This is done using a new input, selectOnClick in dts-select-container.
I'm using this library with some custom drag & drop for a file explorer project. I need to be able to drag & drop all selected items together, however at the current state it is not possible, since starting to drag by clicking on an item will unselect all other items. This fixes this issue.

Thanks again for the awesome work :)

d3lm commented 3 years ago

Hey! Thanks for the PR. I am not sure I understand cause you can use the selectMode, see https://github.com/d3lm/ngx-drag-to-select#dts-select-container. Have you tried to enable that? Or is this not what you are looking for. Do you simply want to disable selection on click but still be able to drag to select items?

I guess selectMode is slightly different cause you can't drag anymore. It's more useful for mobile so I think I do see a value in this new option.

Can you please add tests for this as well and fix the linting issues?

d3lm commented 3 years ago

Strange. I just looked at the linting error and it's a little weird.

sam-eah commented 3 years ago

Do you simply want to disable selection on click but still be able to drag to select items?

Yes, exactly !

d3lm commented 3 years ago

Makes sense. I am down to continue with this PR if you add tests and fix CI.

sam-eah commented 3 years ago

It's a bit strange, I juste tried the cypress tests on my local server and they all passed e2e And I got no errors from linting or styling with the commands style and lint:check either
Do you have any idea ?

d3lm commented 3 years ago

Hm that's indeed very strange. I can look into this on the weekend if I find the time and give it a try on my machine.

d3lm commented 3 years ago

I have also restarted all jobs. Maybe that helps 🤷‍♂️

sam-eah commented 3 years ago

I have also restarted all jobs. Maybe that helps 🤷‍♂️

Okay now all Cypress tests pass!
But the lint task still fails: npm ERR! code ELIFECYCLE I would typically try these commands for this error, but I'm not sure how github CI pipelines handle node modules 😓

sam-eah commented 3 years ago

Hello, any updates ?

d3lm commented 3 years ago

Sorry for my late response. Been super busy lately. I try to have a look this week to see if I can find out why the Lint stage fails and how I can fix it. It also fails in another PR 🤔

sam-eah commented 3 years ago

No worries, thanks for your time!

d3lm commented 3 years ago

Ok I think I have fixed CI. Can you please rebase your PR?

sam-eah commented 3 years ago

Great, hopefully it'll work now!

d3lm commented 3 years ago

I just merged another PR. Please rebase this PR again on latest master. Thanks!

sam-eah commented 3 years ago

Hello, I've added some e2e tests, please tell me if this suits you

sam-eah commented 3 years ago

Awesome! Do you know when a new npm version can be released?

d3lm commented 3 years ago

It's now published. You can grab 4.2.0.

sam-eah commented 3 years ago

Amazing, thanks!