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

Jump to bottom

Fix focus issue when profile selected from nested menu entry #15077

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Fix focus issue when profile selected from nested menu entry #15077

merged 1 commit into from
Apr 25, 2023

Conversation

kkostrzewa
Copy link
Contributor

@kkostrzewa kkostrzewa commented Mar 31, 2023 •
edited by DHowett
Loading

Original bug report #15049
Relates to feature #1571

MenuFlyoutSubItem, when collapsing from profile selection, move focus back to the titlebar.
An extra Closing event handler is needed to keep focus on the command shell.

Closes #15049

@kkostrzewa
MenuFlyoutSubItem profile selection requires an
22bed9a 
extra Closing event to prevent focus from switching
to the tab bar when selecting a profile via mouse.
microsoft#15049
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Mar 31, 2023
DHowett
DHowett reviewed Mar 31, 2023
View reviewed changes
src/cascadia/TerminalApp/TerminalPage.cpp
@@ -855,6 +855,10 @@ namespace winrt::TerminalApp::implementation
newTabFlyout.Opening([this](auto&&, auto&&) {
_FocusCurrentTab(true);
});
// Necessary for fly-out sub items to get focus on a tab before collapsing. Related to #15049
newTabFlyout.Closing([this](auto&&, auto&&) {
_FocusCurrentTab(true);
Copy link
Member

Choose a reason for hiding this comment

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

@carlos-zamora, can you do an accessibility pass over this?

Copy link
Member

Choose a reason for hiding this comment

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

Works great!

DHowett
DHowett requested changes Mar 31, 2023
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

Oh, dang, that's a small fix! Thanks!

I'm blocking only until Carlos (or someone else!) gets a chance to see how this works for accessibility/assistive tech tools like screen readers. I don't want to unduly damage the keyboarding experience 😄

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 31, 2023
@zadjii-msft zadjii-msft added this to the Terminal v1.18 milestone Apr 4, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Apr 24, 2023
@microsoft-github-policy-service
Copy link
Contributor

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

1 similar comment
@microsoft-github-policy-service
Copy link
Contributor

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@zadjii-msft
Copy link
Member

Shhhhhh bot, this is something @carlos-zamora has to do a pass on, not OP

@microsoft-github-policy-service microsoft-github-policy-service bot removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Apr 24, 2023
@zadjii-msft zadjii-msft removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 24, 2023
@carlos-zamora
Copy link
Member

Oh, dang, that's a small fix! Thanks!

I'm blocking only until Carlos (or someone else!) gets a chance to see how this works for accessibility/assistive tech tools like screen readers. I don't want to unduly damage the keyboarding experience 😄

Tested with NVDA. Works great!

@DHowett DHowett changed the title Fix focus problem when profile selected from nested dropdown entry (#15049) Fix focus issue when profile selected from nested menu entry Apr 25, 2023
@DHowett DHowett merged commit def3742 into microsoft:main Apr 25, 2023
@DHowett
Copy link
Member

DHowett commented Apr 25, 2023

@carlos-zamora did you test it with Narrator

@carlos-zamora
Copy link
Member

@carlos-zamora did you test it with Narrator

Yup! Works fine too.

DHowett pushed a commit that referenced this pull request Apr 25, 2023
@kkostrzewa @DHowett
Fix focus issue when profile selected from nested menu entry (#15077)
31dc6d8 
Original bug report #15049
Relates to feature #1571

MenuFlyoutSubItem, when collapsing from profile selection, move focus
back to the titlebar.
An extra Closing event handler is needed to keep focus on the command
shell.

Closes #15049

(cherry picked from commit def3742)
Service-Card-Id: 89002014
Service-Version: 1.17
@zadjii-msft zadjii-msft mentioned this pull request May 12, 2023
Closed
zadjii-msft added a commit that referenced this pull request May 12, 2023
@zadjii-msft
Don't dismiss the command palette when the new tab menu closes
9226086 
Transient UIs are hard.

Regressed in #15077.

Closes #15305
@zadjii-msft zadjii-msft mentioned this pull request May 12, 2023
Merged
DHowett pushed a commit that referenced this pull request May 12, 2023
@zadjii-msft
Don't dismiss the command palette when the new tab menu closes (#15340)
1bf2fcb 
Transient UIs are hard.

Regressed in #15077.

Closes #15305
DHowett pushed a commit that referenced this pull request May 12, 2023
@zadjii-msft @DHowett
Don't dismiss the command palette when the new tab menu closes (#15340)
704be80 
Transient UIs are hard.

Regressed in #15077.

Closes #15305

(cherry picked from commit 1bf2fcb)
Service-Card-Id: 89193705
Service-Version: 1.18
DHowett pushed a commit that referenced this pull request Aug 2, 2023
@zadjii-msft @DHowett
Don't dismiss the command palette when the new tab menu closes (#15340)
c3bdf74 
Transient UIs are hard.

Regressed in #15077.

Closes #15305

(cherry picked from commit 1bf2fcb)
Service-Card-Id: 89899296
Service-Version: 1.17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlos-zamora carlos-zamora carlos-zamora approved these changes

@lhecker lhecker lhecker approved these changes

@DHowett DHowett DHowett approved these changes

Assignees

@DHowett DHowett

Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Terminal v1.18
Development

Successfully merging this pull request may close these issues.

Focus starts on titlebar when opening a profile from a nested dropdown entry
5 participants
@kkostrzewa @zadjii-msft @carlos-zamora @DHowett @lhecker