Skip to content

Commit cce9d1c

Browse files
web-padawanweb-padawan
committed
refactor: do not disable buttons when moving to overflow
1 parent 535a993 commit cce9d1c

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
}
@@ -466,6 +449,12 @@ export const MenuBarMixin = (superClass) =>
466449
this.__renderButtons(items);
467450
}
468451

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

@@ -520,14 +508,6 @@ export const MenuBarMixin = (superClass) =>
520508
item.removeAttribute('tabindex');
521509
}
522510

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

562542
// Save width for buttons with component
563543
btn.style.width = getComputedStyle(btn).width;
564-
btn.disabled = true;
565544
btn.style.visibility = 'hidden';
566545
btn.style.position = 'absolute';
567546
}
@@ -660,7 +639,7 @@ export const MenuBarMixin = (superClass) =>
660639
return html`
661640
<vaadin-menu-bar-button
662641
.item="${itemCopy}"
663-
.disabled="${item.disabled}"
642+
.disabled="${this.disabled || item.disabled}"
664643
role="${this.tabNavigation ? 'button' : 'menuitem'}"
665644
aria-haspopup="${ifDefined(hasChildren ? 'true' : nothing)}"
666645
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)