Skip to content

Conversation

@taooceros
Copy link
Member

resolve #433

We need votes for this pr
@Flow-Launcher/team

@taooceros taooceros changed the title stop auto hiding scroller stop hiding scroller May 19, 2021
@Zeroto521
Copy link
Member

Is there any screenshot to give a comparison?

@taooceros
Copy link
Member Author

Current
image

After
image

@Zeroto521
Copy link
Member

I thought that is a style problem.

There have two UI styles, one is win10 UWP style and another is win7 style.

For FlowLauncher the main UI is win7 style, and the scroller is the UWP style. Different UI styles cause that problem.

For now, I do agree with using the win7 style.

@Zeroto521 Zeroto521 requested review from Zeroto521 and jjw24 May 19, 2021 06:06
Zeroto521
Zeroto521 previously approved these changes May 19, 2021
Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

Please add option to settings to autohide or not the scrollbar. This would be a better option.

@jjw24
Copy link
Member

jjw24 commented May 19, 2021

Also I would consider making it a bit thicker if there is a option in there rather than removing it completely

@taooceros
Copy link
Member Author

Please add option to settings to autohide or not the scrollbar. This would be a better option.

I think the current issue is that new users may not notice that there are some settings at the bottom that they need to scroll down to see. So the default value need to be false?

Also I would consider making it a bit thicker if there is a option in there rather than removing it completely

Not sure, because this is a wrapped feature.

@jjw24
Copy link
Member

jjw24 commented May 19, 2021

Please add option to settings to autohide or not the scrollbar. This would be a better option.

I think the current issue is that new users may not notice that there are some settings at the bottom that they need to scroll down to see. So the default value need to be false?

Yes, default in settings file set to show, then a radio button to toggle between show/hide. Atm this change just show it permanently.

Also I would consider making it a bit thicker if there is a option in there rather than removing it completely

Not sure, because this is a wrapped feature.

Yeah ok probably not possible

@jjw24
Copy link
Member

jjw24 commented Jun 19, 2021

Just added the option to toggle on off auto hide, off being the default.

image

@taooceros Let me know if it's good to merge

@jjw24 jjw24 enabled auto-merge June 19, 2021 11:14
@jjw24 jjw24 added the enhancement New feature or request label Jun 19, 2021
Comment on lines 64 to 75
public bool AutoHideScrollBar
{
get
{
return Settings.AutoHideScrollBar;
}
set
{
Settings.AutoHideScrollBar = value;
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that you directly link the Binding to Settings.AutoHideScrollBar, why is this property needed?

Copy link
Member

Choose a reason for hiding this comment

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

property is for the settings window

Copy link
Member

Choose a reason for hiding this comment

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

because we need to save the state

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think you need this one. WPF will directly modify Settings.AutoHideScrollBar property, which already save the state.

Copy link
Member

Choose a reason for hiding this comment

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

👍 yes you are right, forgot about that. will make the change

Copy link
Member

Choose a reason for hiding this comment

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

@taooceros done, please take another look

@taooceros taooceros added this to the 1.8.0 milestone Jun 21, 2021
@taooceros
Copy link
Member Author

Let me give it a test

Remove default value initialization
@taooceros taooceros force-pushed the StopHidingScroller branch from 1a9f0dd to 77f8a8e Compare June 21, 2021 05:40
@taooceros
Copy link
Member Author

Sorry I was wrong. We still need to backing field because modifying Settings.AutoHideScroller won't trigger the onPropertyChanged.

In this way, the effect will appear once the setting is hit.

@taooceros taooceros requested a review from jjw24 June 21, 2021 05:43
@taooceros taooceros requested a review from Zeroto521 June 21, 2021 05:43
@jjw24 jjw24 merged commit 0b69beb into dev Jun 21, 2021
@jjw24 jjw24 deleted the StopHidingScroller branch June 21, 2021 09:47
pc223 added a commit to pc223/Flow.Launcher that referenced this pull request Jun 21, 2021
The bottom margin (little gap) under General tab of Settings
make it hard to "feel" the space is scrollable or not.

New windows style and websites tend to not have margin in the bottom

Related to: Issue Flow-Launcher#443 and PR Flow-Launcher#440
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Main setting scrollbar hiding additional settings

4 participants