Skip to content

Commit

Permalink
Auto-focus window renamer textbox on open (#12798)
Browse files Browse the repository at this point in the history
Does what it says on the tin. This is maximal BODGE.

`TeachingTip` doesn't provide an `Opened` event.
(microsoft/microsoft-ui-xaml#1607). But we
want to focus the renamer text box when it's opened. We can't do that
immediately, the TextBox technically isn't in the visual tree yet. We
have to wait for it to get added some time after we call IsOpen. How do
we do that reliably? Usually, for this kind of thing, we'd just use a
one-off LayoutUpdated event, as a notification that the TextBox was
added to the tree. HOWEVER:
* The _first_ time this is fired, when the box is _first_ opened,
  yeeting focus doesn't work on the first LayoutUpdated. It does work on
  the second LayoutUpdated. Okay, so we'll wait for two LayoutUpdated
  events, and focus on the second.
* On subsequent opens: We only ever get a single LayoutUpdated. Period.
  But, you can successfully focus it on that LayoutUpdated.

So, we'll keep track of how many LayoutUpdated's we've _ever_ gotten. If
we've had at least 2, then we can focus the text box.

We're also not using a ContentDialog for this, because in Xaml Islands a
text box in a ContentDialog won't receive _any_ keypresses. Fun!

## References
* microsoft/microsoft-ui-xaml#1607
* microsoft/microsoft-ui-xaml#6910
* microsoft/microsoft-ui-xaml#3257
* #9662

## PR Checklist
* [x] Will close out #12021, but that's an a11y bug that needs secondary
  validation
* [x] Closes #11322
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed

Tested manually

(cherry picked from commit b57fe85)
Service-Card-Id: 79978833
Service-Version: 1.12
  • Loading branch information
zadjii-msft authored and DHowett committed Mar 31, 2022
1 parent 013b19c commit 28443c2
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 8 deletions.
48 changes: 42 additions & 6 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -823,18 +823,54 @@ namespace winrt::TerminalApp::implementation
}

_UpdateTeachingTipTheme(WindowRenamer().try_as<winrt::Windows::UI::Xaml::FrameworkElement>());
WindowRenamer().IsOpen(true);

// PAIN: We can't immediately focus the textbox in the TeachingTip. It's
// not technically focusable until it is opened. However, it doesn't
// provide an event to tell us when it is opened. That's tracked in
// microsoft/microsoft-ui-xaml#1607. So for now, the user _needs_ to
// click on the text box manually.
// BODGY: GH#12021
//
// TeachingTip doesn't provide an Opened event.
// (microsoft/microsoft-ui-xaml#1607). But we want to focus the renamer
// text box when it's opened. We can't do that immediately, the TextBox
// technically isn't in the visual tree yet. We have to wait for it to
// get added some time after we call IsOpen. How do we do that reliably?
// Usually, for this kind of thing, we'd just use a one-off
// LayoutUpdated event, as a notification that the TextBox was added to
// the tree. HOWEVER:
// * The _first_ time this is fired, when the box is _first_ opened,
// tossing focus doesn't work on the first LayoutUpdated. It does
// work on the second LayoutUpdated. Okay, so we'll wait for two
// LayoutUpdated events, and focus on the second.
// * On subsequent opens: We only ever get a single LayoutUpdated.
// Period. But, you can successfully focus it on that LayoutUpdated.
//
// So, we'll keep track of how many LayoutUpdated's we've _ever_ gotten.
// If we've had at least 2, then we can focus the text box.
//
// We're also not using a ContentDialog for this, because in Xaml
// Islands a text box in a ContentDialog won't receive _any_ keypresses.
// Fun!
// WindowRenamerTextBox().Focus(FocusState::Programmatic);
_renamerLayoutUpdatedRevoker.revoke();
_renamerLayoutUpdatedRevoker = WindowRenamerTextBox().LayoutUpdated(winrt::auto_revoke, [weakThis = get_weak()](auto&&, auto&&) {
if (auto self{ weakThis.get() })
{
auto& count{ self->_renamerLayoutCount };

// Don't just always increment this, we don't want to deal with overflow situations
if (count < 2)
{
count++;
}

if (count >= 2)
{
self->_renamerLayoutUpdatedRevoker.revoke();
self->WindowRenamerTextBox().Focus(FocusState::Programmatic);
}
}
});
// Make sure to mark that enter was not pressed in the renamer quite
// yet. More details in TerminalPage::_WindowRenamerKeyDown.
_renamerPressedEnter = false;
WindowRenamer().IsOpen(true);

args.Handled(true);
}
Expand Down
26 changes: 24 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3378,6 +3378,26 @@ namespace winrt::TerminalApp::implementation
// of thing with co_return winrt::make<RenameWindowResult>(false).
}

// Method Description:
// - Used to track if the user pressed enter with the renamer open. If we
// immediately focus it after hitting Enter on the command palette, then
// the Enter keydown will dismiss the command palette and open the
// renamer, and then the enter keyup will go to the renamer. So we need to
// make sure both a down and up go to the renamer.
// Arguments:
// - e: the KeyRoutedEventArgs describing the key that was released
// Return Value:
// - <none>
void TerminalPage::_WindowRenamerKeyDown(const IInspectable& /*sender*/,
winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
const auto key = e.OriginalKey();
if (key == Windows::System::VirtualKey::Enter)
{
_renamerPressedEnter = true;
}
}

// Method Description:
// - Manually handle Enter and Escape for committing and dismissing a window
// rename. This is highly similar to the TabHeaderControl's KeyUp handler.
Expand All @@ -3388,16 +3408,18 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_WindowRenamerKeyUp(const IInspectable& sender,
winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
if (e.OriginalKey() == Windows::System::VirtualKey::Enter)
const auto key = e.OriginalKey();
if (key == Windows::System::VirtualKey::Enter && _renamerPressedEnter)
{
// User is done making changes, close the rename box
_WindowRenamerActionClick(sender, nullptr);
}
else if (e.OriginalKey() == Windows::System::VirtualKey::Escape)
else if (key == Windows::System::VirtualKey::Escape)
{
// User wants to discard the changes they made
WindowRenamerTextBox().Text(WindowName());
WindowRenamer().IsOpen(false);
_renamerPressedEnter = false;
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<Toast> _windowIdToast{ nullptr };
std::shared_ptr<Toast> _windowRenameFailedToast{ nullptr };

winrt::Windows::UI::Xaml::Controls::TextBox::LayoutUpdated_revoker _renamerLayoutUpdatedRevoker;
int _renamerLayoutCount{ 0 };
bool _renamerPressedEnter{ false };

winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowDialogHelper(const std::wstring_view& name);

void _ShowAboutDialog();
Expand Down Expand Up @@ -404,6 +408,7 @@ namespace winrt::TerminalApp::implementation

void _WindowRenamerActionClick(const IInspectable& sender, const IInspectable& eventArgs);
void _RequestWindowRename(const winrt::hstring& newName);
void _WindowRenamerKeyDown(const IInspectable& sender, winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);
void _WindowRenamerKeyUp(const IInspectable& sender, winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);

void _UpdateTeachingTipTheme(winrt::Windows::UI::Xaml::FrameworkElement element);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@
IsLightDismissEnabled="True">
<mux:TeachingTip.Content>
<TextBox x:Name="WindowRenamerTextBox"
KeyDown="_WindowRenamerKeyDown"
KeyUp="_WindowRenamerKeyUp"
Text="{x:Bind WindowName, Mode=OneWay}" />
</mux:TeachingTip.Content>
Expand Down

1 comment on commit 28443c2

@github-actions
Copy link

@github-actions github-actions bot commented on 28443c2 Mar 31, 2022

Choose a reason for hiding this comment

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

@check-spelling-bot Report

Unrecognized words, please review:

  • hwinsta
  • Lsa
  • lsass
  • ofstream
  • tmpdir
  • UOI
  • USEROBJECTFLAGS
  • winsta
  • winstamin
  • WSF
Previously acknowledged words that are now absent adaa carlos coffgroup coffgrp datetime DCompile deconstructed devicefamily dpg eae emplate GENPROFILE GTR guardxfg HHmm Hostx installationpath MMdd MSDL pgorepro pgort PGU redistributable sid SPACEBAR Timeline timelines Unregister UWA UWAs vcvarsall xfg xIcon yIcon zamora
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:microsoft/terminal.git repository
on the release-1.12 branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/28443c2302fe6715f47b51c780cc4a9896123b21.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/terminal/comments/70071552" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ If you see a bunch of garbage

If it relates to a ...

well-formed pattern

See if there's a pattern that would match it.

If not, try writing one and adding it to a patterns/{file}.txt.

Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

Note that patterns can't match multiline strings.

binary-ish string

Please add a file path to the excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

Please sign in to comment.