Skip to content

fix(aria-expanded) on submenues add aria prop #79775

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

Merged
SteVen Batten (sbatten) merged 4 commits into from Nov 26, 2019
Merged

fix(aria-expanded) on submenues add aria prop #79775

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea11bd0
fix(aria-expended) on submenues add aria prop
Aug 25, 2019
7a2adef
(aria-expanded) on sub menu creation :star:
Aug 29, 2019
f63c4cf
Merge branch 'master' into pr/georgebatalinski/79775
sbatten Oct 17, 2019
a2aa8de
add missing places for setting to false
sbatten Oct 17, 2019
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
Prev Previous commit
add missing places for setting to false 
credit to @jeanp413 for calling out this duplication
  • Loading branch information
@sbatten
sbatten committed Oct 17, 2019
commit a2aa8de309cbed3937672ba5664943860a808273
16 changes: 2 additions & 14 deletions src/vs/base/browser/ui/menu/menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,13 +775,7 @@ class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {

this.parentData.parent.focus();

if (this.parentData.submenu) {
this.parentData.submenu.dispose();
this.parentData.submenu = undefined;
}

this.submenuDisposables.clear();
this.submenuContainer = undefined;
this.cleanupExistingSubmenu(true);
}
}));

Expand All @@ -796,13 +790,7 @@ class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {
this.submenuDisposables.add(this.parentData.submenu.onDidCancel(() => {
this.parentData.parent.focus();

if (this.parentData.submenu) {

@sbatten SteVen Batten (sbatten) Oct 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Jean Pierre (@jeanp413) credit to you for calling this out in your PR. In this PR it became necessary to fix without further duplicating code.

this.parentData.submenu.dispose();
this.parentData.submenu = undefined;
}

this.submenuDisposables.clear();
this.submenuContainer = undefined;
this.cleanupExistingSubmenu(true);
}));

this.parentData.submenu.focus(selectFirstItem);
Expand Down