[PR #12275] Fix broken reset button on some profile settings #28949

Open
opened 2026-01-31 09:31:49 +00:00 by claunia · 0 comments
Owner

Original Pull Request: https://github.com/microsoft/terminal/pull/12275

State: closed
Merged: Yes


Summary of the Pull Request

This fixes a bug where several settings would not show the reset button. The root cause of this issue is two fold:

  1. Hooking up CurrentXXX
    • GETSET_BINDABLE_ENUM_SETTING was hooked up to the settings model profile object instead of the view model profile object. Since the settings model has no PropertyChanged system, any changes were directly being applied to the setting, but not notifying the view model (and thus, the view, by extension) to update themselves.
    • This fix required me to slightly modify the macro. Rather than using two parameters (object and function name), I used one parameter (path to getter/setter).
  2. Responding to the PropertyChanged notifications
    • Now that we're actually dispatching the PropertyChanged notifications, we need to actually respond to them. This behavior was defined in Profiles::OnNavigatedTo() in the PropertyChanged() handler. Funny enough, that code was still there, it just didn't do anything because it was trying to notify that Profiles::CurrentXXX changed. This is invalid because CurrentXXX got moved to ProfileViewModel.
    • The fix here was pretty easy. Just move the property changed handler to ProfileViewModel's PropertyChanged handler that is defined in the ctor.

References

Bug introduced in #11877

Validation Steps Performed

Profile termination behavior
Bell notification style
Text antialiasing
Scrollbar visibility

**Original Pull Request:** https://github.com/microsoft/terminal/pull/12275 **State:** closed **Merged:** Yes --- ## Summary of the Pull Request This fixes a bug where several settings would not show the reset button. The root cause of this issue is two fold: 1. Hooking up `CurrentXXX` - `GETSET_BINDABLE_ENUM_SETTING` was hooked up to the **settings** model profile object instead of the **view** model profile object. Since the settings model has no `PropertyChanged` system, any changes were directly being applied to the setting, but not notifying the view model (and thus, the view, by extension) to update themselves. - This fix required me to slightly modify the macro. Rather than using two parameters (object and function name), I used one parameter (path to getter/setter). 2. Responding to the `PropertyChanged` notifications - Now that we're actually dispatching the `PropertyChanged` notifications, we need to actually respond to them. This behavior was defined in `Profiles::OnNavigatedTo()` in the `PropertyChanged()` handler. Funny enough, that code was still there, it just didn't do anything because it was trying to notify that `Profiles::CurrentXXX` changed. This is invalid because `CurrentXXX` got moved to `ProfileViewModel`. - The fix here was pretty easy. Just move the property changed handler to `ProfileViewModel`'s `PropertyChanged` handler that is defined in the ctor. ## References Bug introduced in #11877 ## Validation Steps Performed ✅ Profile termination behavior ✅ Bell notification style ✅ Text antialiasing ✅ Scrollbar visibility
claunia added the pull-request label 2026-01-31 09:31:49 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#28949