Skip to content

Commit c22ba0c

Browse files
web-padawanweb-padawan
committed
refactor: do not disable buttons when moving to overflow
1 parent 114fb7f commit c22ba0c

File tree

2 files changed

+11
-96
lines changed

2 files changed

+11
-96
lines changed

‎packages/menu-bar/src/vaadin-menu-bar-mixin.js

+9-30
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ export const MenuBarMixin = (superClass) =>
216216
'_themeChanged(_theme, _overflow, _container)',
217217
'__hasOverflowChanged(_hasOverflow, _overflow)',
218218
'__i18nChanged(__effectiveI18n, _overflow)',
219-
'_menuItemsChanged(items, _overflow, _container)',
219+
'__updateButtons(items, disabled, _overflow, _container)',
220220
'_reverseCollapseChanged(reverseCollapse, _overflow, _container)',
221221
'_tabNavigationChanged(tabNavigation, _overflow, _container)',
222222
];
@@ -381,23 +381,6 @@ export const MenuBarMixin = (superClass) =>
381381
this.__detectOverflow();
382382
}
383383

384-
/**
385-
* Override method inherited from `DisabledMixin`
386-
* to update the `disabled` property for the buttons
387-
* whenever the property changes on the menu bar.
388-
*
389-
* @param {boolean} newValue the new disabled value
390-
* @param {boolean} oldValue the previous disabled value
391-
* @override
392-
* @protected
393-
*/
394-
_disabledChanged(newValue, oldValue) {
395-
super._disabledChanged(newValue, oldValue);
396-
if (oldValue !== newValue) {
397-
this.__updateButtonsDisabled(newValue);
398-
}
399-
}
400-
401384
/**
402385
* A callback for the `_theme` property observer.
403386
* It propagates the host theme to the buttons and the sub menu.
@@ -456,7 +439,7 @@ export const MenuBarMixin = (superClass) =>
456439
}
457440

458441
/** @private */
459-
_menuItemsChanged(items, overflow, container) {
442+
__updateButtons(items, disabled, overflow, container) {
460443
if (!overflow || !container) {
461444
return;
462445
}
@@ -467,6 +450,12 @@ export const MenuBarMixin = (superClass) =>
467450
this.__detectOverflow();
468451
}
469452

453+
if (disabled !== this._oldDisabled) {
454+
this._oldDisabled = disabled;
455+
this.__renderButtons(items);
456+
overflow.toggleAttribute('disabled', disabled);
457+
}
458+
470459
const subMenu = this._subMenu;
471460
if (subMenu && subMenu.opened) {
472461
const button = subMenu._overlayElement.positionTarget;
@@ -499,7 +488,6 @@ export const MenuBarMixin = (superClass) =>
499488
/** @private */
500489
__restoreButtons(buttons) {
501490
buttons.forEach((button) => {
502-
button.disabled = (button.item && button.item.disabled) || this.disabled;
503491
button.style.visibility = '';
504492
button.style.position = '';
505493
button.style.width = '';
@@ -522,14 +510,6 @@ export const MenuBarMixin = (superClass) =>
522510
item.removeAttribute('tabindex');
523511
}
524512

525-
/** @private */
526-
__updateButtonsDisabled(disabled) {
527-
this._buttons.forEach((btn) => {
528-
// Disable the button if the entire menu-bar is disabled or the item alone is disabled
529-
btn.disabled = disabled || (btn.item && btn.item.disabled);
530-
});
531-
}
532-
533513
/** @private */
534514
__updateOverflow(items) {
535515
this._overflow.item = { children: items };
@@ -563,7 +543,6 @@ export const MenuBarMixin = (superClass) =>
563543

564544
// Save width for buttons with component
565545
btn.style.width = getComputedStyle(btn).width;
566-
btn.disabled = true;
567546
btn.style.visibility = 'hidden';
568547
btn.style.position = 'absolute';
569548
}
@@ -662,7 +641,7 @@ export const MenuBarMixin = (superClass) =>
662641
return html`
663642
<vaadin-menu-bar-button
664643
.item="${itemCopy}"
665-
.disabled="${item.disabled}"
644+
.disabled="${this.disabled || item.disabled}"
666645
role="${this.tabNavigation ? 'button' : 'menuitem'}"
667646
aria-haspopup="${ifDefined(hasChildren ? 'true' : nothing)}"
668647
aria-expanded="${ifDefined(hasChildren ? 'false' : nothing)}"

‎packages/menu-bar/test/overflow.test.js

+2-66
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,15 @@ describe('overflow', () => {
3030
`);
3131
menu = wrapper.querySelector('vaadin-menu-bar');
3232
await nextRender(menu);
33-
menu.items = [
34-
{ text: 'Item 1' },
35-
{ text: 'Item 2' },
36-
{ text: 'Item 3' },
37-
{ text: 'Item 4' },
38-
{ text: 'Item 5', disabled: true },
39-
];
33+
menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }, { text: 'Item 3' }, { text: 'Item 4' }, { text: 'Item 5' }];
4034
await nextUpdate(menu);
4135
buttons = menu._buttons;
4236
overflow = buttons[buttons.length - 1];
4337
});
4438

4539
it('should show overflow button and hide the buttons which do not fit', () => {
4640
assertHidden(buttons[2]);
47-
expect(buttons[2].disabled).to.be.true;
4841
assertHidden(buttons[3]);
49-
expect(buttons[3].disabled).to.be.true;
5042
expect(overflow.hasAttribute('hidden')).to.be.false;
5143
});
5244

@@ -69,9 +61,7 @@ describe('overflow', () => {
6961
menu.style.width = '350px';
7062
await nextResize(menu);
7163
assertVisible(buttons[2]);
72-
expect(buttons[2].disabled).to.not.be.true;
7364
assertVisible(buttons[3]);
74-
expect(buttons[3].disabled).to.not.be.true;
7565
expect(overflow.item.children.length).to.equal(1);
7666
expect(overflow.item.children[0]).to.deep.equal(menu.items[4]);
7767
});
@@ -81,9 +71,7 @@ describe('overflow', () => {
8171
menu.style.width = '350px';
8272
await nextResize(menu);
8373
assertVisible(buttons[2]);
84-
expect(buttons[2].disabled).to.not.be.true;
8574
assertVisible(buttons[3]);
86-
expect(buttons[3].disabled).to.not.be.true;
8775
expect(overflow.item.children.length).to.equal(1);
8876
expect(overflow.item.children[0]).to.deep.equal(menu.items[4]);
8977
});
@@ -92,7 +80,6 @@ describe('overflow', () => {
9280
menu.style.width = '150px';
9381
await nextResize(menu);
9482
assertHidden(buttons[1]);
95-
expect(buttons[1].disabled).to.be.true;
9683
expect(overflow.item.children.length).to.equal(4);
9784
expect(overflow.item.children[0]).to.deep.equal(menu.items[1]);
9885
expect(overflow.item.children[1]).to.deep.equal(menu.items[2]);
@@ -105,7 +92,6 @@ describe('overflow', () => {
10592
menu.style.width = '150px';
10693
await nextResize(menu);
10794
assertHidden(buttons[1]);
108-
expect(buttons[1].disabled).to.be.true;
10995
expect(overflow.item.children.length).to.equal(4);
11096
expect(overflow.item.children[0]).to.deep.equal(menu.items[1]);
11197
expect(overflow.item.children[1]).to.deep.equal(menu.items[2]);
@@ -117,11 +103,8 @@ describe('overflow', () => {
117103
menu.style.width = 'auto';
118104
await nextResize(menu);
119105
assertVisible(buttons[2]);
120-
expect(buttons[2].disabled).to.not.be.true;
121106
assertVisible(buttons[3]);
122-
expect(buttons[3].disabled).to.not.be.true;
123107
assertVisible(buttons[4]);
124-
expect(buttons[4].disabled).to.be.true;
125108
expect(overflow.hasAttribute('hidden')).to.be.true;
126109
expect(overflow.item.children.length).to.equal(0);
127110
});
@@ -176,22 +159,6 @@ describe('overflow', () => {
176159
await nextResize(menu);
177160

178161
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
179-
expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
180-
});
181-
182-
it('should set tabindex -1 on the overflow menu in tab navigation', async () => {
183-
menu.tabNavigation = true;
184-
buttons[0].focus();
185-
arrowRight(buttons[0]);
186-
187-
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
188-
expect(buttons[1].getAttribute('tabindex')).to.equal('0');
189-
190-
menu.style.width = '150px';
191-
await nextResize(menu);
192-
193-
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
194-
expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
195162
});
196163

197164
it('should set the aria-label of the overflow button according to the i18n of the menu bar', async () => {
@@ -216,15 +183,10 @@ describe('overflow', () => {
216183

217184
it('should show overflow button and hide the buttons which do not fit', () => {
218185
assertHidden(buttons[0]);
219-
expect(buttons[0].disabled).to.be.true;
220186
assertHidden(buttons[1]);
221-
expect(buttons[1].disabled).to.be.true;
222187
assertHidden(buttons[2]);
223-
expect(buttons[2].disabled).to.be.true;
224188
assertVisible(buttons[3]);
225-
expect(buttons[3].disabled).to.be.false;
226189
assertVisible(buttons[4]);
227-
expect(buttons[4].disabled).to.be.true;
228190

229191
expect(overflow.hasAttribute('hidden')).to.be.false;
230192
});
@@ -242,15 +204,10 @@ describe('overflow', () => {
242204
menu.reverseCollapse = false;
243205
await nextUpdate(menu);
244206
assertVisible(buttons[0]);
245-
expect(buttons[0].disabled).to.be.false;
246207
assertVisible(buttons[1]);
247-
expect(buttons[1].disabled).to.be.false;
248208
assertHidden(buttons[2]);
249-
expect(buttons[2].disabled).to.be.true;
250209
assertHidden(buttons[3]);
251-
expect(buttons[3].disabled).to.be.true;
252210
assertHidden(buttons[4]);
253-
expect(buttons[4].disabled).to.be.true;
254211
});
255212
});
256213
});
@@ -337,13 +294,7 @@ describe('overflow', () => {
337294

338295
container.style.width = '250px';
339296

340-
menu.items = [
341-
{ text: 'Item 1' },
342-
{ text: 'Item 2' },
343-
{ text: 'Item 3' },
344-
{ text: 'Item 4' },
345-
{ text: 'Item 5', disabled: true },
346-
];
297+
menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }, { text: 'Item 3' }, { text: 'Item 4' }, { text: 'Item 5' }];
347298
await nextRender(menu);
348299
buttons = menu._buttons;
349300
overflow = buttons[buttons.length - 1];
@@ -357,31 +308,16 @@ describe('overflow', () => {
357308
container.style.width = '150px';
358309
await nextResize(menu);
359310
assertHidden(buttons[2]);
360-
expect(buttons[2].disabled).to.be.true;
361311
assertHidden(buttons[3]);
362-
expect(buttons[3].disabled).to.be.true;
363312

364313
container.style.width = '400px';
365314
await nextResize(menu);
366315
assertVisible(buttons[2]);
367-
expect(buttons[2].disabled).to.not.be.true;
368316
assertVisible(buttons[3]);
369-
expect(buttons[3].disabled).to.not.be.true;
370317
assertVisible(buttons[4]);
371-
expect(buttons[4].disabled).to.be.true;
372318
expect(overflow.hasAttribute('hidden')).to.be.true;
373319
expect(overflow.item.children.length).to.equal(0);
374320
});
375-
376-
it('should keep buttons disabled when resizing', async () => {
377-
menu.disabled = true;
378-
await nextUpdate(menu);
379-
container.style.width = '150px';
380-
await nextResize(menu);
381-
buttons.forEach((btn) => {
382-
expect(btn.disabled).to.be.true;
383-
});
384-
});
385321
});
386322

387323
describe('parent resize', () => {

0 commit comments

Comments
 (0)