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.

Sign up for GitHub

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

fix(nested): don't toggle disabled children #20962

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

fix(nested): don't toggle disabled children #20962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3da719
fix(nested): don't toggle disabled children
johnleider Feb 5, 2025
1ec83ea
test(nested): add test for disabled items
johnleider Feb 5, 2025
00493b8
fix: retain existing argument order
KaelWD Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/vuetify/src/components/VList/VListGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const makeVListGroupProps = propsFactory({
type: IconValue,
default: '$collapse',
},
disabled: Boolean,
expandIcon: {
type: IconValue,
default: '$expand',
Expand All @@ -60,7 +61,7 @@ export const VListGroup = genericComponent<VListGroupSlots>()({
props: makeVListGroupProps(),

setup (props, { slots }) {
const { isOpen, open, id: _id } = useNestedItem(toRef(props, 'value'), true)
const { isOpen, open, id: _id } = useNestedItem(toRef(props, 'value'), toRef(props, 'disabled'), true)
const id = computed(() => `v-list-group--id-${String(_id.value)}`)
const list = useList()
const { isBooted } = useSsrBoot()
Expand Down
4 changes: 2 additions & 2 deletions packages/vuetify/src/components/VList/VListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { genOverlays, makeVariantProps, useVariant } from '@/composables/variant
import { Ripple } from '@/directives/ripple'

// Utilities
import { computed, onBeforeMount, watch } from 'vue'
import { computed, onBeforeMount, toRef, watch } from 'vue'
import { deprecate, EventProp, genericComponent, propsFactory, useRender } from '@/util'

// Types
Expand Down Expand Up @@ -128,7 +128,7 @@ export const VListItem = genericComponent<VListItemSlots>()({
parent,
openOnSelect,
id: uid,
} = useNestedItem(id, false)
} = useNestedItem(id, toRef(props, 'disabled'), false)
const list = useList()
const isActive = computed(() =>
props.active !== false &&
Expand Down
82 changes: 81 additions & 1 deletion packages/vuetify/src/composables/nested/__tests__/selectStrategies.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ describe('selectStrategies', () => {
['2', '1'],
['3', '1'],
])

let selected = new Map()

selected = strategy.select({
id: '2',
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -32,6 +32,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -58,6 +59,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['1', 'on'],
Expand All @@ -68,6 +70,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['2', 'on'],
Expand All @@ -91,6 +94,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'on'],
]),
Expand All @@ -103,6 +107,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand All @@ -128,6 +133,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand Down Expand Up @@ -157,6 +163,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -165,6 +172,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -190,6 +198,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['1', 'on'],
Expand All @@ -200,6 +209,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['2', 'on'],
Expand All @@ -223,6 +233,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'on'],
]),
Expand All @@ -235,6 +246,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand All @@ -260,6 +272,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand Down Expand Up @@ -289,6 +302,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -297,6 +311,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -322,6 +337,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map())

Expand All @@ -330,6 +346,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['2', 'on'],
Expand All @@ -353,6 +370,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'on'],
]),
Expand All @@ -365,6 +383,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand All @@ -390,6 +409,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand Down Expand Up @@ -419,6 +439,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -427,6 +448,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -453,6 +475,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map())

Expand All @@ -461,6 +484,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['2', 'on'],
Expand All @@ -484,6 +508,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'on'],
]),
Expand All @@ -496,6 +521,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand All @@ -521,6 +547,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['2', 'on'],
]),
Expand Down Expand Up @@ -553,6 +580,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['1', 'indeterminate'],
Expand Down Expand Up @@ -584,6 +612,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected: new Map(),
})).toEqual(new Map([
['1', 'indeterminate'],
Expand Down Expand Up @@ -616,6 +645,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'indeterminate'],
['3', 'on'],
Expand Down Expand Up @@ -651,6 +681,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand All @@ -664,6 +695,7 @@ describe('selectStrategies', () => {
value: true,
children,
parents,
disabled: new Set(),
selected,
})

Expand Down Expand Up @@ -696,6 +728,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'indeterminate'],
['3', 'indeterminate'],
Expand All @@ -714,6 +747,7 @@ describe('selectStrategies', () => {
value: false,
children,
parents,
disabled: new Set(),
selected: new Map([
['1', 'on'],
['2', 'on'],
Expand All @@ -731,5 +765,51 @@ describe('selectStrategies', () => {
['6', 'on'],
]))
})

it('should not select disabled children when parent is selected', () => {
Copy link
Member

Choose a reason for hiding this comment

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

return-object is usually vulnerable when nested composable has changes, so probably better add a test case to cover return-object as well

const strategy = classicSelectStrategy(false)

const children = new Map([
['1', ['2', '3']],
['3', ['4', '5']],
])

const parents = new Map([
['2', '1'],
['3', '1'],
['4', '3'],
['5', '3'],
])

const disabled = new Set(['3'])

let selected = new Map()

selected = strategy.select({
id: '1',
value: true,
children,
parents,
disabled,
selected,
})

expect(selected.has('3')).toBe(false)

disabled.delete('3')
disabled.add('2')

selected = strategy.select({
id: '1',
value: true,
children,
parents,
disabled,
selected: new Map(),
})

expect(selected.has('2')).toBe(false)
expect(selected.has('3')).toBe(true)
})
})
})
Loading