-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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.
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
Convert the About and Close All Tabs dialogs to xaml #5140
Conversation
This commit replaces the hand-coded Close and About dialogs with Xaml markup that uses Uids for localization and data bindings/event bindings for critical information and actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah the smell of fresh cut code in the morning
@@ -58,6 +59,8 @@ namespace winrt::TerminalApp::implementation | |||
TYPED_EVENT(Initialized, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs); | |||
|
|||
private: | |||
friend struct TerminalPageT<TerminalPage>; // for Xaml to bind events |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this is because that one event handler is private
, I see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dustin: *deletes code* it works better now
Hello @carlos-zamora! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Summary of the Pull Request
This pull request replaces about a hundred lines of manual xaml DOM code with a few lines of actual xaml, and wires up bound properties and event handlers in the good and correct way.
As part of this change, I've replaced the giant TextBlock in the about dialog with StackPanels, and replaced the Hyperlinks with HyperlinkButtons. This is in line with other platform applications.
URLs are not localizable resources, so I moved them into the about dialog's xaml. Per #5138, we'll likely change them so that they get localization for "free" (dispatching based on the browser's language, without having to localize the URL in the application).