-
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
Add UI for adding, renaming, and deleting a color scheme #8403
Changes from 1 commit
9493ca0
51ba054
a305c0d
91d2148
b87f6d6
bde5d5e
09677fd
29578d3
e002752
0ce05b5
c14824e
8213cf1
788e685
b0b5fc1
f5e9d95
3b04ba9
44d16b8
8a10d29
d3d128d
52b0915
15b4565
1ef9f13
9c4d77a
b757e82
22805bd
78545a0
1a36ca6
e2dcd16
9fc79d5
60b154b
c8fd30e
96cbbb7
666e179
18ac211
81ca24b
6f56738
c0bca31
314baf9
aef3336
a9dbe8f
a8d52c1
140784f
644f732
a6eb2da
c58321a
7c992c2
36b2e29
48ad994
f1af7ab
41d6cdd
ffa2252
851fb02
c731b0f
f80ee27
5ca0db1
79a99bd
3cf7552
ddc4593
1a224cc
05d1a7b
36fef26
0943ef8
f9fc986
879ed1b
cbcda1a
62aa0ce
2f747a7
5770d23
540fbb3
16ca947
6e41aec
1a11ae0
722bf70
4e3b6e4
0dedf38
9ff3a45
812c36c
af17c59
bac748e
ada4a0b
a20138c
9daf48e
0635849
1ce0e55
443f515
b6874f6
a030056
93158e7
58c81ff
3a38333
106c1e3
7133d05
91dc221
67249d9
b38df24
f2379e6
69693f6
d676103
f829d5a
6bdedbe
7315977
e6a69fd
8165371
a24e49f
937a6d1
8953246
98377fa
c240d76
ac6c951
9204c88
2846605
f7bb0fb
516a047
a3d6695
cc8dec1
dd2ef1c
a2ebc74
541f46c
fd8db96
4436762
556be58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,19 +235,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
|
||
void MainPage::_Navigate(const Editor::ProfileViewModel& profile) | ||
{ | ||
auto state{ winrt::make_self<ProfilePageNavigationState>(profile, _settingsClone.GlobalSettings().ColorSchemes(), *this) }; | ||
auto state{ winrt::make<ProfilePageNavigationState>(profile, _settingsClone.GlobalSettings().ColorSchemes(), *this) }; | ||
|
||
// You cannot delete the Windows Powershell and CMD profiles | ||
const auto myGuid{ profile.Guid() }; | ||
const winrt::guid pwshGuid{ ::Microsoft::Console::Utils::GuidFromString(L"{61c54bbd-c2c6-5271-96e7-009a87ff44bf}") }; | ||
const winrt::guid cmdGuid{ ::Microsoft::Console::Utils::GuidFromString(L"{0caa0dad-35be-5f56-a8ff-afceeeaa6101}") }; | ||
if (myGuid != pwshGuid && myGuid != cmdGuid) | ||
{ | ||
auto pfnDeleteProfile{ std::bind(&MainPage::_DeleteProfile, this, profile) }; | ||
state->SetDeleteProfileCallback(pfnDeleteProfile); | ||
} | ||
// Add an event handler for when the user wants to delete a profile. | ||
state.DeleteProfile({ this, &MainPage::_DeleteProfile }); | ||
|
||
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), *state); | ||
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), state); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not the hugest fan of having the INCOMING STATE contain an event binding, but... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. It does feel a bit weird to add the event binding to the state. But I also don't know what else we can do haha. Open to suggestions! |
||
} | ||
|
||
void MainPage::OpenJsonTapped(IInspectable const& /*sender*/, Windows::UI::Xaml::Input::TappedRoutedEventArgs const& /*args*/) | ||
|
@@ -334,10 +327,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
return profileNavItem; | ||
} | ||
|
||
void MainPage::_DeleteProfile(Editor::ProfileViewModel& profile) | ||
void MainPage::_DeleteProfile(const IInspectable /*sender*/, const Editor::DeleteProfileEventArgs& args) | ||
{ | ||
// Delete profile from settings model | ||
const auto guid{ profile.Guid() }; | ||
const auto guid{ args.ProfileGuid() }; | ||
auto profileList{ _settingsClone.AllProfiles() }; | ||
for (uint32_t i = 0; i < profileList.Size(); ++i) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,37 @@ static const std::set<std::wstring_view> ProfileGeneratorNamespaces{ | |
|
||
namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | ||
{ | ||
bool ProfilePageNavigationState::CanDeleteProfile() const | ||
{ | ||
const auto guid{ Profile().Guid() }; | ||
const std::wstring_view source{ Profile().Source() }; | ||
if (IsBaseLayer()) | ||
{ | ||
return false; | ||
} | ||
else if (InBoxProfileGuids.find(guid) != InBoxProfileGuids.end()) | ||
{ | ||
// in-box profile | ||
return false; | ||
} | ||
else if (LegacyDynamicProfileGuids.find(guid) != LegacyDynamicProfileGuids.end() || | ||
ProfileGeneratorNamespaces.find(source) != ProfileGeneratorNamespaces.end()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not just "has a source"? That covers all of these cases without us having to hardcode a list of sources. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing to "has a source". Curious, what's the point of the legacy dynamic profile guids again? I ran into them and thought they were important for a situation like this. |
||
{ | ||
// dynamic profile | ||
return false; | ||
} | ||
else | ||
{ | ||
return true; | ||
} | ||
} | ||
|
||
void ProfilePageNavigationState::DeleteProfile() | ||
{ | ||
auto deleteProfileArgs{ winrt::make_self<DeleteProfileEventArgs>(_Profile.Guid()) }; | ||
_DeleteProfileHandlers(*this, *deleteProfileArgs); | ||
} | ||
|
||
Profiles::Profiles() : | ||
_ColorSchemeList{ single_threaded_observable_vector<ColorScheme>() } | ||
{ | ||
|
@@ -265,4 +296,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation | |
{ | ||
return _State.Profile().CursorShape() == TerminalControl::CursorStyle::Vintage; | ||
} | ||
|
||
// -------------------------------- WinRT Events --------------------------------- | ||
// Winrt events need a method for adding a callback to the event and removing the callback. | ||
// These macros will define them both for you. | ||
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(ProfilePageNavigationState, DeleteProfile, _DeleteProfileHandlers, Editor::ProfilePageNavigationState, Editor::DeleteProfileEventArgs); | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
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.
woah wait why is this a
make
and not amake_self
? Just curiousThere 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.
No reason, really. I've been using
winrt::make
for all the other navigation states so I guess to be consistent