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

feat(VDataTable): Add shift key logic for multiple selection #11921

Merged
merged 8 commits into from Apr 24, 2021

Conversation

Aramis13
Copy link
Contributor

@Aramis13 Aramis13 commented Jul 23, 2020

Description

Add a listener for shift key press.
Add logic for selecting multiple check-boxes.

Motivation and Context

resolves #9958.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • [ x] The PR title is no longer than 64 characters.
  • [ x] The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • [ x] My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@MajesticPotatoe MajesticPotatoe added C: VDataTable VDatatable T: feature A new feature labels Nov 10, 2020
Copy link
Member

@nekosaur nekosaur left a comment

Choose a reason for hiding this comment

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

There are a few behaviors here that I feel are not standard when it comes to shift selecting.

  1. Select last entry without holding shift. Then select first entry while holding shift. My expectation here is that it would select from last entry to first entry, but that is not the case.

  2. Select first entry without holding shift. Then select last entry while holding shift. The entire page is select. Then while still holding shift, select a middle entry. Here I would expect that everything from middle entry to the last entry would be deselected, but the opposite happens where everything from first entry to middle entry is deselected.

Comment on lines 215 to 230
if (index < this.lastEntry) {
for (let i = index; i <= this.lastEntry; i++) {
const currentItem = this.selectableItems[i]
const key = getObjectValueByPath(currentItem, this.itemKey)
if (value) selection[key] = currentItem
else delete selection[key]
emit && this.$emit('item-selected', { currentItem, value })
}
} else if (index > this.lastEntry) {
for (let i = this.lastEntry; i <= index; i++) {
const currentItem = this.selectableItems[i]
const key = getObjectValueByPath(currentItem, this.itemKey)
if (value) selection[key] = currentItem
else delete selection[key]
emit && this.$emit('item-selected', { currentItem, value })
}
Copy link
Member

Choose a reason for hiding this comment

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

Try to refactor this into something more DRY. The only difference between the ifs is the for loop definition

@KaelWD KaelWD added this to the v2.5.0 milestone Dec 23, 2020
@KaelWD KaelWD requested a review from nekosaur December 23, 2020 15:49
@KaelWD KaelWD merged commit bb252c1 into vuetifyjs:dev Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VDataTable VDatatable T: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants