Fix issue where the slider background would shine through gaps between slider and thumb #3673
Conversation
…n slider and thumb
ghost
added
the
|
Its always a good idea to add comments directly in the code explaining why things like this are done. Also a link to the GitHub issue is convention. This will allow it to be removed in the future if ever composition fixes these rendering glitches. |
|
Good idea @robloo ! Added comments now explaining why we use a negative margin there. Btw, this would likely not be something that will be fixed by composition but rather the controls team as the rect/thumb combination is unfortunate here. But still, if this get's fixed it's good to have linked to that so we can find it easier later on. |
|
can you please add the before screenshot as well to the description ? Thanks |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ranjeshj
added
area-Slider
team-Controls
Sure thing, updated the description. |
|
I am guessing the updated slider design may fix this, but just bringing this to your attention using the upcoming new Design Tokens and colours... Specifically the Disabled state.
New designs which may fix things |
|
This bug is fixed with the new slider visual update in PR now. Link here. |
|
So this PR can be closed then? |
|
@chingucoding Karen's change is going into the Feature/token-experiment branch. I think this fix will still end up being valuable in master. |
| Using a negative margin to hide the round corners that would otherwise leave a gap between the rect and the thumb. | ||
| See https://github.com/microsoft/microsoft-ui-xaml/issues/1765 for more context. | ||
| --> | ||
| <Rectangle x:Name="HorizontalDecreaseRect" Fill="{TemplateBinding Foreground}" Grid.Row="1" Margin="0,0,-5,0" |
There was a problem hiding this comment.
5 [](start = 137, length = 1)
Does this need to scale based on the value of SliderHorizontalThumbWidth?
There was a problem hiding this comment.
Yes in theory it should. Problem is, if we use converters, it wouldn't pick up user made changes due to the fact that binding inside resource dictionaries are only evaluated once if I recall correctly.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@chingucoding do you want to confirm my merge? |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Merge looks good, thank you @StephenLPeters. |
|
@chingucoding can you take a look at the merge conflict? |
|
Not sure why it says "empty files" now, but merge conflict is resolved @StephenLPeters. |
|
@chingucoding unfortunately, this fix doesn't work properly - it will cause thumb not be able to reach a 100 visually because it will be "pulled back" by 5 pixels :) |
|
Alrighty, thanks @beervoley . |
|
Closing this as #5353 was merged. |
Description
Using a negative margin, the the track extends underneath the thumb hiding the gaps. The fix is not great but better than nothing.
Motivation and Context
Fixes #1765
How Has This Been Tested?
Tested manually.
Screenshots (if appropriate):
Before
After