Skip to content

Commit

Permalink
Introduce read-only panes (#8867)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Introduces read-only panes.
When pane is marked as read-only:
1. Attempt to provide user input results in a warning
2. Attempt to close pane - shows dialog
3. Attempt to close hosting tab shows dialog
4. The hosting tab has no close button

## PR Checklist
* [x] Closes #6981
* [x] CLA signed. 
* [ ] Tests added/passed
* [ ] Documentation updated - not yet.
* [x] Schema updated.
* [ ] I've discussed this with core contributors already. 

## Detailed Description of the Pull Request / Additional comments
1. The readonly  logic implemented in `TermControl`
(and prevents any send input)
2. Special handling is required to allow key-bindings
3. The "close-readonly" protections are in TerminalPage.
4. The indication that the pane is readonly is done using lock glyph
5. The indication that the tab contains readonly pane
is done by hiding the close button of the tab
6. The readonly mode is enabled by keyboard shortcut
(the followup might add this to the context menu)

## Validation Steps Performed
  • Loading branch information
Don-Vito authored Feb 8, 2021
1 parent a3c8bea commit 3230b18
Show file tree
Hide file tree
Showing 26 changed files with 293 additions and 37 deletions.
1 change: 1 addition & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
"toggleFocusMode",
"toggleFullscreen",
"togglePaneZoom",
"toggleReadOnlyMode",
"toggleShaderEffects",
"wt",
"unbound"
Expand Down
34 changes: 23 additions & 11 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,17 @@ namespace winrt::TerminalApp::implementation
args.Handled(true);
}

void TerminalPage::_HandleTogglePaneReadOnly(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (const auto activeTab{ _GetFocusedTabImpl() })
{
activeTab->TogglePaneReadOnly();
}

args.Handled(true);
}

void TerminalPage::_HandleScrollUpPage(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
Expand Down Expand Up @@ -465,18 +476,20 @@ namespace winrt::TerminalApp::implementation
return;
}

// Remove tabs after the current one
while (_tabs.Size() > index + 1)
// Since _RemoveTab is asynchronous, create a snapshot of the tabs we want to remove
std::vector<winrt::TerminalApp::TabBase> tabsToRemove;
if (index > 0)
{
_RemoveTabViewItemByIndex(_tabs.Size() - 1);
std::copy(begin(_tabs), begin(_tabs) + index, std::back_inserter(tabsToRemove));
}

// Remove all of them leading up to the selected tab
while (_tabs.Size() > 1)
if (index + 1 < _tabs.Size())
{
_RemoveTabViewItemByIndex(0);
std::copy(begin(_tabs) + index + 1, end(_tabs), std::back_inserter(tabsToRemove));
}

_RemoveTabs(tabsToRemove);

actionArgs.Handled(true);
}
}
Expand All @@ -502,11 +515,10 @@ namespace winrt::TerminalApp::implementation
return;
}

// Remove tabs after the current one
while (_tabs.Size() > index + 1)
{
_RemoveTabViewItemByIndex(_tabs.Size() - 1);
}
// Since _RemoveTab is asynchronous, create a snapshot of the tabs we want to remove
std::vector<winrt::TerminalApp::TabBase> tabsToRemove;
std::copy(begin(_tabs) + index + 1, end(_tabs), std::back_inserter(tabsToRemove));
_RemoveTabs(tabsToRemove);

// TODO:GH#7182 For whatever reason, if you run this action
// when the tab that's currently focused is _before_ the `index`
Expand Down
14 changes: 12 additions & 2 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,11 @@ void Pane::_ControlLostFocusHandler(winrt::Windows::Foundation::IInspectable con
// - <none>
void Pane::Close()
{
// Fire our Closed event to tell our parent that we should be removed.
_ClosedHandlers(nullptr, nullptr);
if (!_isClosing.exchange(true))
{
// Fire our Closed event to tell our parent that we should be removed.
_ClosedHandlers(nullptr, nullptr);
}
}

// Method Description:
Expand Down Expand Up @@ -2085,6 +2088,13 @@ std::optional<SplitState> Pane::PreCalculateAutoSplit(const std::shared_ptr<Pane
FAIL_FAST();
}

// Method Description:
// - Returns true if the pane or one of its descendants is read-only
bool Pane::ContainsReadOnly() const
{
return _IsLeaf() ? _control.ReadOnly() : (_firstChild->ContainsReadOnly() || _secondChild->ContainsReadOnly());
}

DEFINE_EVENT(Pane, GotFocus, _GotFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);
DEFINE_EVENT(Pane, LostFocus, _LostFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);
DEFINE_EVENT(Pane, PaneRaiseBell, _PaneRaiseBellHandlers, winrt::Windows::Foundation::EventHandler<bool>);
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class Pane : public std::enable_shared_from_this<Pane>
void Id(uint16_t id) noexcept;
void FocusPane(const uint16_t id);

bool ContainsReadOnly() const;

WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);
DECLARE_EVENT(LostFocus, _LostFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);
Expand Down Expand Up @@ -118,6 +120,8 @@ class Pane : public std::enable_shared_from_this<Pane>

Borders _borders{ Borders::None };

std::atomic<bool> _isClosing{ false };

bool _zoomed{ false };

bool _IsLeaf() const noexcept;
Expand Down
12 changes: 12 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,18 @@
<data name="CloseAllDialog.Title" xml:space="preserve">
<value>Do you want to close all tabs?</value>
</data>
<data name="CloseReadOnlyDialog.CloseButtonText" xml:space="preserve">
<value>Close anyway</value>
</data>
<data name="CloseReadOnlyDialog.PrimaryButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="CloseReadOnlyDialog.Title" xml:space="preserve">
<value>Warning</value>
</data>
<data name="CloseReadOnlyDialog.Content" xml:space="preserve">
<value>You are about to close a read-only terminal. Do you wish to continue?</value>
</data>
<data name="LargePasteDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ namespace winrt::TerminalApp::implementation
_BreakIntoDebuggerHandlers(*this, eventArgs);
break;
}
case ShortcutAction::TogglePaneReadOnly:
{
_TogglePaneReadOnlyHandlers(*this, eventArgs);
break;
}
default:
return false;
}
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ namespace winrt::TerminalApp::implementation
TYPED_EVENT(TabSearch, TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
TYPED_EVENT(MoveTab, TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
TYPED_EVENT(BreakIntoDebugger, TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
TYPED_EVENT(TogglePaneReadOnly, TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
// clang-format on

private:
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.idl
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,6 @@ namespace TerminalApp
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> TabSearch;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> MoveTab;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> BreakIntoDebugger;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> TogglePaneReadOnly;
}
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TabBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace winrt::TerminalApp::implementation

OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Title, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Icon, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(bool, ReadOnly, _PropertyChangedHandlers, false);
GETSET_PROPERTY(winrt::Microsoft::UI::Xaml::Controls::TabViewItem, TabViewItem, nullptr);

OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::FrameworkElement, Content, _PropertyChangedHandlers, nullptr);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TabBase.idl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ namespace TerminalApp
{
String Title { get; };
String Icon { get; };
Boolean ReadOnly { get; };

Microsoft.UI.Xaml.Controls.TabViewItem TabViewItem { get; };
Windows.UI.Xaml.FrameworkElement Content { get; };
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TabHeaderControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace winrt::TerminalApp::implementation
OBSERVABLE_GETSET_PROPERTY(bool, IsProgressRingIndeterminate, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(bool, BellIndicator, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(uint32_t, ProgressValue, _PropertyChangedHandlers);
OBSERVABLE_GETSET_PROPERTY(bool, IsReadOnlyActive, _PropertyChangedHandlers);

private:
bool _receivedKeyDown{ false };
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TabHeaderControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace TerminalApp
Boolean IsProgressRingIndeterminate { get; set; };
Boolean BellIndicator { get; set; };
UInt32 ProgressValue { get; set; };
Boolean IsReadOnlyActive { get; set; };

TabHeaderControl();
void BeginRename();
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalApp/TabHeaderControl.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ the MIT License. See LICENSE in the project root for license information. -->
Glyph="&#xE8A3;"
FontSize="12"
Margin="0,0,8,0"/>
<FontIcon x:Name="HeaderLockIcon"
FontFamily="Segoe MDL2 Assets"
Visibility="{x:Bind IsReadOnlyActive, Mode=OneWay}"
Glyph="&#xE72E;"
FontSize="12"
Margin="0,0,8,0"/>
<TextBlock x:Name="HeaderTextBlock"
Visibility="Visible"
Text="{x:Bind Title, Mode=OneWay}"/>
Expand Down
100 changes: 82 additions & 18 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,17 @@ namespace winrt::TerminalApp::implementation
co_return ContentDialogResult::None;
}

// Method Description:
// - Displays a dialog for warnings found while closing the terminal tab marked as read-only
winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> TerminalPage::_ShowCloseReadOnlyDialog()
{
if (auto presenter{ _dialogPresenter.get() })
{
co_return co_await presenter.ShowDialog(FindName(L"CloseReadOnlyDialog").try_as<WUX::Controls::ContentDialog>());
}
co_return ContentDialogResult::None;
}

// Method Description:
// - Displays a dialog to warn the user about the fact that the text that
// they are trying to paste contains the "new line" character which can
Expand Down Expand Up @@ -1055,6 +1066,7 @@ namespace winrt::TerminalApp::implementation
_actionDispatch->TabSearch({ this, &TerminalPage::_HandleOpenTabSearch });
_actionDispatch->MoveTab({ this, &TerminalPage::_HandleMoveTab });
_actionDispatch->BreakIntoDebugger({ this, &TerminalPage::_HandleBreakIntoDebugger });
_actionDispatch->TogglePaneReadOnly({ this, &TerminalPage::_HandleTogglePaneReadOnly });
}

// Method Description:
Expand Down Expand Up @@ -1164,7 +1176,7 @@ namespace winrt::TerminalApp::implementation

// Method Description:
// - Look for the index of the input tabView in the tabs vector,
// and call _RemoveTabViewItemByIndex
// and call _RemoveTab
// Arguments:
// - tabViewItem: the TabViewItem in the TabView that is being removed.
void TerminalPage::_RemoveTabViewItem(const MUX::Controls::TabViewItem& tabViewItem)
Expand All @@ -1173,16 +1185,35 @@ namespace winrt::TerminalApp::implementation
if (_tabView.TabItems().IndexOf(tabViewItem, tabIndexFromControl))
{
// If IndexOf returns true, we've actually got an index
_RemoveTabViewItemByIndex(tabIndexFromControl);
auto tab{ _tabs.GetAt(tabIndexFromControl) };
_RemoveTab(tab);
}
}

// Method Description:
// - Removes the tab (both TerminalControl and XAML)
// Arguments:
// - tabIndex: the index of the tab to be removed
void TerminalPage::_RemoveTabViewItemByIndex(uint32_t tabIndex)
// - tab: the tab to remove
winrt::Windows::Foundation::IAsyncAction TerminalPage::_RemoveTab(winrt::TerminalApp::TabBase tab)
{
if (tab.ReadOnly())
{
ContentDialogResult warningResult = co_await _ShowCloseReadOnlyDialog();

// The primary action is canceling the removal
if (warningResult == ContentDialogResult::Primary)
{
co_return;
}
}

uint32_t tabIndex{};
if (!_tabs.IndexOf(tab, tabIndex))
{
// The tab is already removed
co_return;
}

// We use _removing flag to suppress _OnTabSelectionChanged events
// that might get triggered while removing
_removing = true;
Expand All @@ -1192,11 +1223,10 @@ namespace winrt::TerminalApp::implementation

// Removing the tab from the collection should destroy its control and disconnect its connection,
// but it doesn't always do so. The UI tree may still be holding the control and preventing its destruction.
auto tab{ _tabs.GetAt(tabIndex) };
tab.Shutdown();

uint32_t mruIndex;
if (_mruTabs.IndexOf(_tabs.GetAt(tabIndex), mruIndex))
uint32_t mruIndex{};
if (_mruTabs.IndexOf(tab, mruIndex))
{
_mruTabs.RemoveAt(mruIndex);
}
Expand Down Expand Up @@ -1265,6 +1295,8 @@ namespace winrt::TerminalApp::implementation
_rearrangeFrom = std::nullopt;
_rearrangeTo = std::nullopt;
}

co_return;
}

// Method Description:
Expand Down Expand Up @@ -1540,26 +1572,54 @@ namespace winrt::TerminalApp::implementation
{
if (auto index{ _GetFocusedTabIndex() })
{
_RemoveTabViewItemByIndex(*index);
auto tab{ _tabs.GetAt(*index) };
_RemoveTab(tab);
}
}

// Method Description:
// - Close the currently focused pane. If the pane is the last pane in the
// tab, the tab will also be closed. This will happen when we handle the
// tab's Closed event.
void TerminalPage::_CloseFocusedPane()
winrt::fire_and_forget TerminalPage::_CloseFocusedPane()
{
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
_UnZoomIfNeeded();
terminalTab->ClosePane();

auto pane = terminalTab->GetActivePane();

if (const auto pane{ terminalTab->GetActivePane() })
{
if (const auto control{ pane->GetTerminalControl() })
{
if (control.ReadOnly())
{
ContentDialogResult warningResult = co_await _ShowCloseReadOnlyDialog();

// The primary action is canceling the action
if (warningResult == ContentDialogResult::Primary)
{
co_return;
}

// Clean read-only mode to prevent additional prompt if closing the pane triggers closing of a hosting tab
if (control.ReadOnly())
{
control.ToggleReadOnly();
}
}

pane->Close();
}
}
}
else if (auto index{ _GetFocusedTabIndex() })
{
if (_tabs.GetAt(*index).try_as<TerminalApp::SettingsTab>())
const auto tab{ _tabs.GetAt(*index) };
if (tab.try_as<TerminalApp::SettingsTab>())
{
_RemoveTabViewItemByIndex(*index);
_RemoveTab(tab);
}
}
}
Expand All @@ -1581,17 +1641,21 @@ namespace winrt::TerminalApp::implementation
}
}

_CloseAllTabs();
// Since _RemoveTab is asynchronous, create a snapshot of the tabs we want to remove
std::vector<winrt::TerminalApp::TabBase> tabsToRemove;
std::copy(begin(_tabs), end(_tabs), std::back_inserter(tabsToRemove));
_RemoveTabs(tabsToRemove);
}

// Method Description:
// - Remove all the tabs opened and the terminal will terminate
// on its own when the last tab is closed.
void TerminalPage::_CloseAllTabs()
// - Closes provided tabs one by one
// Arguments:
// - tabs - tabs to remove
winrt::fire_and_forget TerminalPage::_RemoveTabs(const std::vector<winrt::TerminalApp::TabBase> tabs)
{
while (_tabs.Size() != 0)
for (auto& tab : tabs)
{
_RemoveTabViewItemByIndex(0);
co_await _RemoveTab(tab);
}
}

Expand Down
Loading

0 comments on commit 3230b18

Please sign in to comment.