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.

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

Show clipboard contents in multiple line paste warning dialog #8744

Merged
6 commits merged into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@
<data name="MultiLinePasteDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="MultiLinePasteDialog.Content" xml:space="preserve">
<data name="MultiLineWarningText.Text" xml:space="preserve">
Copy link
Member

Choose a reason for hiding this comment

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

i wish we didn't have to change the key for this. I know why we have to, but this will trigger a re-localization for a string that is almost exactly the same.

<value>You are about to paste text that contains multiple lines. If you paste this text into your shell, it may result in the unexpected execution of commands. Do you wish to continue?</value>
</data>
<data name="MultiLinePasteDialog.PrimaryButtonText" xml:space="preserve">
Expand Down Expand Up @@ -527,4 +527,7 @@
<data name="NoticeWarning" xml:space="preserve">
<value>Warning</value>
</data>
<data name="ClipboardTextHeader.Text" xml:space="preserve">
<value>Clipboard contents (preview):</value>
</data>
</root>
10 changes: 10 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1957,6 +1957,13 @@ namespace winrt::TerminalApp::implementation
{
co_await winrt::resume_foreground(Dispatcher());

// We have to initialize the dialog here to be able to change the text of the text block within it
FindName(L"MultiLinePasteDialog").try_as<WUX::Controls::ContentDialog>();
ClipboardText().Text(text);

// The vertical offset on the scrollbar does not reset automatically, so reset it manually
ClipboardContentScrollViewer().ScrollToVerticalOffset(0);

ContentDialogResult warningResult;
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
if (warnMultiLine)
{
Expand All @@ -1967,6 +1974,9 @@ namespace winrt::TerminalApp::implementation
warningResult = co_await _ShowLargePasteWarningDialog();
}

// Clear the clipboard text so it doesn't lie around in memory
ClipboardText().Text(L"");

if (warningResult != ContentDialogResult::Primary)
{
// user rejected the paste
Expand Down
20 changes: 20 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,26 @@ the MIT License. See LICENSE in the project root for license information. -->
x:Name="MultiLinePasteDialog"
x:Uid="MultiLinePasteDialog"
DefaultButton="Primary">
<StackPanel>
<TextBlock
x:Uid="MultiLineWarningText"
TextWrapping="Wrap">
</TextBlock>
<TextBlock
x:Uid="ClipboardTextHeader"
Margin="0,16,0,0">
</TextBlock>
<ScrollViewer
Margin="0,8,0,0"
x:Name="ClipboardContentScrollViewer"
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to name infrastructural components unless we refer to them in code 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We actually refer to this one! To reset the scroll offset (the scroll offset does not reset automatically in between calls to open the dialog)

MaxHeight="100">
<TextBlock
x:Name="ClipboardText"
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
TextWrapping="Wrap"
FontFamily="Cascadia Mono">
</TextBlock>
</ScrollViewer>
</StackPanel>
</ContentDialog>

<ContentDialog
Expand Down