Skip to content

Commit 2590bd6

Browse files
web-padawanweb-padawan
committed
refactor: do not disable buttons when moving to overflow
1 parent c414a7f commit 2590bd6

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.
@@ -455,7 +438,7 @@ export const MenuBarMixin = (superClass) =>
455438
}
456439

457440
/** @private */
458-
_menuItemsChanged(items, overflow, container) {
441+
__updateButtons(items, disabled, overflow, container) {
459442
if (!overflow || !container) {
460443
return;
461444
}
@@ -465,6 +448,12 @@ export const MenuBarMixin = (superClass) =>
465448
this.__renderButtons(items);
466449
}
467450

451+
if (disabled !== this._oldDisabled) {
452+
this._oldDisabled = disabled;
453+
this.__renderButtons(items);
454+
overflow.toggleAttribute('disabled', disabled);
455+
}
456+
468457
const subMenu = this._subMenu;
469458
if (subMenu && subMenu.opened) {
470459
const button = subMenu._overlayElement.positionTarget;
@@ -497,7 +486,6 @@ export const MenuBarMixin = (superClass) =>
497486
/** @private */
498487
__restoreButtons(buttons) {
499488
buttons.forEach((button) => {
500-
button.disabled = (button.item && button.item.disabled) || this.disabled;
501489
button.style.visibility = '';
502490
button.style.position = '';
503491

@@ -519,14 +507,6 @@ export const MenuBarMixin = (superClass) =>
519507
item.removeAttribute('tabindex');
520508
}
521509

522-
/** @private */
523-
__updateButtonsDisabled(disabled) {
524-
this._buttons.forEach((btn) => {
525-
// Disable the button if the entire menu-bar is disabled or the item alone is disabled
526-
btn.disabled = disabled || (btn.item && btn.item.disabled);
527-
});
528-
}
529-
530510
/** @private */
531511
__updateOverflow(items) {
532512
this._overflow.item = { children: items };
@@ -560,7 +540,6 @@ export const MenuBarMixin = (superClass) =>
560540

561541
// Save width for buttons with component
562542
btn.style.width = getComputedStyle(btn).width;
563-
btn.disabled = true;
564543
btn.style.visibility = 'hidden';
565544
btn.style.position = 'absolute';
566545
}
@@ -659,7 +638,7 @@ export const MenuBarMixin = (superClass) =>
659638
return html`
660639
<vaadin-menu-bar-button
661640
.item="${itemCopy}"
662-
.disabled="${item.disabled}"
641+
.disabled="${this.disabled || item.disabled}"
663642
role="${this.tabNavigation ? 'button' : 'menuitem'}"
664643
aria-haspopup="${ifDefined(hasChildren ? 'true' : nothing)}"
665644
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
});
@@ -160,22 +143,6 @@ describe('overflow', () => {
160143
await nextResize(menu);
161144

162145
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
163-
expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
164-
});
165-
166-
it('should set tabindex -1 on the overflow menu in tab navigation', async () => {
167-
menu.tabNavigation = true;
168-
buttons[0].focus();
169-
arrowRight(buttons[0]);
170-
171-
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
172-
expect(buttons[1].getAttribute('tabindex')).to.equal('0');
173-
174-
menu.style.width = '150px';
175-
await nextResize(menu);
176-
177-
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
178-
expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
179146
});
180147

181148
it('should set the aria-label of the overflow button according to the i18n of the menu bar', async () => {
@@ -200,15 +167,10 @@ describe('overflow', () => {
200167

201168
it('should show overflow button and hide the buttons which do not fit', () => {
202169
assertHidden(buttons[0]);
203-
expect(buttons[0].disabled).to.be.true;
204170
assertHidden(buttons[1]);
205-
expect(buttons[1].disabled).to.be.true;
206171
assertHidden(buttons[2]);
207-
expect(buttons[2].disabled).to.be.true;
208172
assertVisible(buttons[3]);
209-
expect(buttons[3].disabled).to.be.false;
210173
assertVisible(buttons[4]);
211-
expect(buttons[4].disabled).to.be.true;
212174

213175
expect(overflow.hasAttribute('hidden')).to.be.false;
214176
});
@@ -226,15 +188,10 @@ describe('overflow', () => {
226188
menu.reverseCollapse = false;
227189
await nextUpdate(menu);
228190
assertVisible(buttons[0]);
229-
expect(buttons[0].disabled).to.be.false;
230191
assertVisible(buttons[1]);
231-
expect(buttons[1].disabled).to.be.false;
232192
assertHidden(buttons[2]);
233-
expect(buttons[2].disabled).to.be.true;
234193
assertHidden(buttons[3]);
235-
expect(buttons[3].disabled).to.be.true;
236194
assertHidden(buttons[4]);
237-
expect(buttons[4].disabled).to.be.true;
238195
});
239196
});
240197
});
@@ -321,13 +278,7 @@ describe('overflow', () => {
321278

322279
container.style.width = '250px';
323280

324-
menu.items = [
325-
{ text: 'Item 1' },
326-
{ text: 'Item 2' },
327-
{ text: 'Item 3' },
328-
{ text: 'Item 4' },
329-
{ text: 'Item 5', disabled: true },
330-
];
281+
menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }, { text: 'Item 3' }, { text: 'Item 4' }, { text: 'Item 5' }];
331282
await nextRender(menu);
332283
buttons = menu._buttons;
333284
overflow = buttons[buttons.length - 1];
@@ -341,31 +292,16 @@ describe('overflow', () => {
341292
container.style.width = '150px';
342293
await nextResize(menu);
343294
assertHidden(buttons[2]);
344-
expect(buttons[2].disabled).to.be.true;
345295
assertHidden(buttons[3]);
346-
expect(buttons[3].disabled).to.be.true;
347296

348297
container.style.width = '400px';
349298
await nextResize(menu);
350299
assertVisible(buttons[2]);
351-
expect(buttons[2].disabled).to.not.be.true;
352300
assertVisible(buttons[3]);
353-
expect(buttons[3].disabled).to.not.be.true;
354301
assertVisible(buttons[4]);
355-
expect(buttons[4].disabled).to.be.true;
356302
expect(overflow.hasAttribute('hidden')).to.be.true;
357303
expect(overflow.item.children.length).to.equal(0);
358304
});
359-
360-
it('should keep buttons disabled when resizing', async () => {
361-
menu.disabled = true;
362-
await nextUpdate(menu);
363-
container.style.width = '150px';
364-
await nextResize(menu);
365-
buttons.forEach((btn) => {
366-
expect(btn.disabled).to.be.true;
367-
});
368-
});
369305
});
370306

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

0 commit comments

Comments
 (0)