-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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 support for scroll bar 'always' setting #14047
Conversation
HECK yeah! I'm so excited to get into this. |
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.
Well, you made this look very straightforward. Seriously, thank you. Couldn't've asked for more. 😄
{ | ||
for (auto foundState : group.States()) | ||
{ | ||
if (foundState.Name() == L"ExpandedWithoutAnimation") |
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.
This is for safety's sake, right? So GoToStateCore
doesn't blow up if this state doesn't exist, and so we fall back to the default behavior in case of the same?
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.
Right.
@msftbot merge this in 24 hours |
Hello @DHowett! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
@msftbot merge this in 5 minutes |
Hello @DHowett! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
I cannot see this feature in the release of https://github.com/microsoft/terminal/releases/tag/v1.15.2874.0 - I tried a Windows 10 Build - the "always" setting is not yet supported. How can I identify in which Release this change will be integrated? Somehow a PR which was merged after this is included in the Changelog of the Release. |
@volviq 1.15 is our current "stable" build, so we'll frequently backport changes to that release for bugfixes and reliability's sake. This feature is going to be released in 1.17. A helpful msftbot will come through and post here when that release is available, and include the official version number. |
Thanks for the quick reply. Do you have nightly builds? #1738 indicates not. Any easy way to get this feature without building myself? |
Nope. It's something we discussed as a team meeting just this week, but it'll require a while to set up. Gotta figure out hosting pipelines, etc. Probably not something that we'll get to before 1.17 comes out next month. |
🎉 Handy links: |
This fixes #3454 by adding support for an "always" mode for the scroll bar.
This change uses a custom VisualStateManager to keep the scroll bar from collapsing if the profile is using the 'always' setting.
Validation Steps Performed
Tried updating settings.json directly and using the UI and making sure the scroll bar behaves as expected.
Closes #3454