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

[Windows] Clamp images inside buttons to button width/height #22276

Merged
merged 14 commits into from May 15, 2024
Merged

[Windows] Clamp images inside buttons to button width/height #22276

merged 14 commits into from May 15, 2024

Conversation

Foda
Copy link
Member

@Foda Foda commented May 7, 2024

Description of Change

This PR forces images inside buttons to always fit inside the available space, rather than expand be clipped

Issues Fixed

Fixes #21683 #21970

@Foda Foda requested review from mattleibow and PureWeen May 7, 2024 17:15
@Foda Foda requested a review from a team as a code owner May 7, 2024 17:15
@dotnet-policy-service dotnet-policy-service bot added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 7, 2024
jsuarezruiz
jsuarezruiz reviewed May 9, 2024
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

Have done tests with some samples like Issue18242.
image
The behavior looks good so far, but, in the snapshot from Issue21513Test, the first image seems to be clipped:
image

Is this expected?

@mattleibow
Copy link
Member

As @jsuarezruiz noticed, the image is clipped. But if you look at all the other images, they are all clipping now. Or rather, still clipping. Should they?

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented May 10, 2024 •
edited

After the latest changes related with spacing, the snapshot for the test Issue21513Test needs changes.

Snapshot different than baseline: Issue21513Test.png (0.62% difference)

image

@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
mattleibow
mattleibow approved these changes May 15, 2024
View reviewed changes
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Looks good!

Not sure if there is value in a device test for Windows that picks a few sizes/options and then renders it on screen. The test would be to make sure the position of things are where they are expected to be.

@Foda Foda merged commit 374e9dd into main May 15, 2024
49 checks passed
@Foda Foda deleted the foda/imgbuttonresizemain branch May 15, 2024 18:07
@PureWeen PureWeen mentioned this pull request May 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattleibow mattleibow mattleibow approved these changes

@jsuarezruiz jsuarezruiz jsuarezruiz approved these changes

@PureWeen PureWeen Awaiting requested review from PureWeen

Assignees
No one assigned
Labels
area-controls-button Button, ImageButton platform/windows 🪟
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Windows] Image on button not scaling down for all scenarios when dimensions are less than button
5 participants
@Foda @mattleibow @jsuarezruiz @Eilon @PureWeen