Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Scrollbar] Fix carat spacing to edges #4889

Merged
merged 10 commits into from
Apr 29, 2021
Merged

[Scrollbar] Fix carat spacing to edges #4889

merged 10 commits into from
Apr 29, 2021

Conversation

almedina-ms
Copy link
Contributor

@almedina-ms almedina-ms commented Apr 23, 2021 •
edited
Loading

Fix the spacing between the carat and the scrollbar edge

Description

Adds margins to all carats to match the design

Adds padding to the RepeatButton templates to avoid layout cycles

Motivation and Context

VSO 32319998

How Has This Been Tested?

Approval

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Apr 23, 2021
ranjeshj
ranjeshj approved these changes Apr 23, 2021
View reviewed changes
Copy link
Contributor

@ranjeshj ranjeshj left a comment

Choose a reason for hiding this comment

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

:shipit:

@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters added area-ScrollBar area-UIDesign UI Design, styling team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Apr 23, 2021
@beervoley
Copy link
Contributor

@almedina-ms please merge master, I think you need to get fresh visual verification files

@almedina-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@almedina-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@almedina-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj
Copy link
Contributor

@almedina-ms can you move the arrow by one more pixel and share screenshot with design ?

ranjeshj
ranjeshj requested changes Apr 27, 2021
View reviewed changes
Copy link
Contributor

@ranjeshj ranjeshj left a comment

Choose a reason for hiding this comment

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

This is causing a layout cycle which might be a known issue. @RBrid @beervoley might have hit the same issue before.

@almedina-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@almedina-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj requested a review from RBrid April 28, 2021 18:02
@beervoley
Copy link
Contributor

@almedina-ms won't it change the gap between the thumb and the arrow buttons?
it will become smaller by 4 px for each individual arrow :)

@almedina-ms
Copy link
Contributor Author

@almedina-ms won't it change the gap between the thumb and the arrow buttons?
it will become smaller by 4 px for each individual arrow :)

yes, we are aware of that issue but we ended up going with this as we should not modify that gap

@almedina-ms almedina-ms merged commit 89bc99d into main Apr 29, 2021
@almedina-ms almedina-ms deleted the user/almedina-ms/ScrollBar_VisualUpdates branch April 29, 2021 19:09
beervoley pushed a commit that referenced this pull request May 3, 2021
@almedina-ms @beervoley
[Scrollbar] Fix carat spacing to edges (#4889)
5b2b256 
* Update spacing on edges

* Clean code to remove changes

* Small change reverted

* Udpate expected result

* Force failure for expected results

* Fix carat to edge spacing with no test error

* Revert tree view tests failures

* Update verification files
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Dec 17, 2023
@Kinnara
Port microsoft/microsoft-ui-xaml#4889
d338abb
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Dec 17, 2023
@Kinnara
Port microsoft/microsoft-ui-xaml#4889 (#656)
e6120b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RBrid RBrid RBrid approved these changes

@ranjeshj ranjeshj ranjeshj approved these changes

Assignees
No one assigned
Labels
area-ScrollBar area-UIDesign UI Design, styling team-Controls Issue for the Controls team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@almedina-ms @ranjeshj @beervoley @RBrid @StephenLPeters