fix(lib): start selection inbetween select items

[PhOder]

This commit fixes the issue of not being able to start a selection if mousedown occured on neither Host element nor select item element but inbetween. The issue occurs if SelectItems are not direct children of the SelectContainer.

Fixes #144

[PhOder]

Hi @d3lm thank you for this great library! :)

As this is my first PR in Github I'm sure there's going to be a couple of things I should fix. So please just let me know if you've got any feedback 👍

[d3lm]

Thanks! And also thanks for your PR, really appreciate it. I ll have a look at your PR 👍

[officiallyvenkatesan]

Hi @d3lm, why this PR not yet accepted. I checked this and it's working as expected. I need this change for my project. So give this feature ASAP. Thank you!

[d3lm]

Hey there! If you really need this you always have the option to fork it. I am not maintaining this library in a full-time nor part-time capacity at the moment and I have many other responsibilities so I ask for a bit of understanding. I just didn't have the time to look at this PR yet. I am afraid that being pushy won't help much here.

[PhOder]

Hey there! Thanks for having a look at my PR. I'll try to have a look in the evening =)

Dominic Elm @.***> schrieb am So., 28. Juli 2024, 10:34:

@.**** requested changes on this pull request.

Thanks for the PR. Left some minor remarks.

In cypress/integration/dragging.spec.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694177161 :

@@ -91,6 +93,82 @@ describe('Dragging', () => { .dispatch('mouseup'); }); }); +

• describe('selection with dragOverItems set to false', () => {

⬇️ Suggested change

• describe('selection with dragOverItems set to false', () => {
• describe('Selection with dragOverItems set to false', () => {

---

In cypress/integration/dragging.spec.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694177524 :

• disableDragOverItems().then(() => {
• getDesktopExample().within(() => {
• cy.getSelectItem(0)
• .dispatch('mousedown', { button: 0 })
• .getSelectItem(6, 'end')
• .dispatch('mousemove')
• .shouldSelect([1])
• .getSelectBox()
• .then(shouldBeInvisible)
• .@.***')
• .dispatch('mouseup');
• });
• });
• });
• it('should start selection in element inbetween SelectContainer and SelectItem', () => {

⬇️ Suggested change

• it('should start selection in element inbetween SelectContainer and SelectItem', () => {
• it('should start selection on element inbetween SelectContainer and SelectItem', () => {

---

In cypress/integration/dragging.spec.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694177615 :

• .wait(16)
• .trigger('mousedown', 210, 50, { button: 0 })
• .wait(16)
• .getSelectItem(6)
• .dispatch('mousemove')
• .shouldSelect([2, 3, 6, 7])
• .getSelectBox()
• .then(shouldBeVisible)
• .@.***')
• .dispatch('mouseup');
• });
• });
• });
• });
• describe('selection with selectOnClick set to false', () => {

⬇️ Suggested change

• describe('selection with selectOnClick set to false', () => {
• describe('Selection with selectOnClick set to false', () => {

---

In cypress/integration/dragging.spec.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694177718 :

• disableSelectOnClick().then(() => {
• getDesktopExample().within(() => {
• cy.getSelectItem(0)
• .dispatch('mousedown', { button: 0 })
• .getSelectItem(6, 'end')
• .dispatch('mousemove')
• .shouldSelect([])
• .getSelectBox()
• .then(shouldBeInvisible)
• .@.***')
• .dispatch('mouseup');
• });
• });
• });
• it('should start selection in element inbetween SelectContainer and SelectItem', () => {

⬇️ Suggested change

• it('should start selection in element inbetween SelectContainer and SelectItem', () => {
• it('should start selection on element inbetween SelectContainer and SelectItem', () => {

---

In cypress/integration/dragging.spec.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694178411 :

• .shouldSelect([1])
• .getSelectBox()
• .then(shouldBeInvisible)
• .@.***')
• .dispatch('mouseup');
• });
• });
• });
• it('should start selection in element inbetween SelectContainer and SelectItem', () => {
• disableDragOverItems().then(() => {
• getDesktopExample().within(() => {
• cy.get('mat-grid-list')
• .as('end')
• .scrollIntoView()
• .wait(16)

Why are all these manual wait points necessary? Generally I'd say it's not ideal especially because Cypress has automatic wait points built-in in almost all APIs. Can you elaborate why these are needed?

In projects/ngx-drag-to-select/src/lib/select-container.component.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694179149 :

@@ -680,4 +683,13 @@ export class SelectContainerComponent implements AfterViewInit, OnDestroy, After

 return null;

} +

• private _isClickOutsideSelectableItem(element: EventTarget): boolean {
• if (!(element instanceof HTMLElement)) return false;

⬇️ Suggested change

• if (!(element instanceof HTMLElement)) return false;
• if (!(element instanceof HTMLElement)) {
• return false;
• }

---

In projects/ngx-drag-to-select/src/lib/select-container.component.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694179207 :

@@ -680,4 +683,13 @@ export class SelectContainerComponent implements AfterViewInit, OnDestroy, After

 return null;

} +

• private _isClickOutsideSelectableItem(element: EventTarget): boolean {
• if (!(element instanceof HTMLElement)) return false;
• if (element === this.host) return true;

Add a block here too.

In projects/ngx-drag-to-select/src/lib/select-container.component.ts https://github.com/d3lm/ngx-drag-to-select/pull/167#discussion_r1694179274 :

@@ -680,4 +683,13 @@ export class SelectContainerComponent implements AfterViewInit, OnDestroy, After

 return null;

} +

• private _isClickOutsideSelectableItem(element: EventTarget): boolean {
• if (!(element instanceof HTMLElement)) return false;
• if (element === this.host) return true;
• if (this._selectableItemsNative.includes(element)) return false;

Newline before and add a block.

— Reply to this email directly, view it on GitHub https://github.com/d3lm/ngx-drag-to-select/pull/167#pullrequestreview-2203421712, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACI4ZRXXWFZCALI4H77LDWDZOSUILAVCNFSM6AAAAABLP6A5TGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMBTGQZDCNZRGI . You are receiving this because you authored the thread.Message ID: @.***>

[PhOder]

Thanks for the review @d3lm I hope everything checks out okay now :)

[officiallyvenkatesan]

Hello @d3lm I hope this message finds you well. I am a dedicated user of this package. Firstly, I would like to express my sincere gratitude for your continuous efforts in maintaining and improving this invaluable resource. I am writing to bring to your attention a recent pull request submitted by @PhOder. This update is particularly important for my current project as it would provide enhanced user experience. I fully understand and respect the careful consideration required for reviewing and merging pull requests to ensure the highest standards of quality and stability. Given the significant positive impact these changes could have on my project, I kindly request your consideration in reviewing and accepting this pull request. Thank you very much for your attention and understanding. I look forward to your positive response.