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.

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 ScaleType.CENTER_CROP #322

Merged
merged 4 commits into from
Mar 7, 2022

Conversation

scana
Copy link
Contributor

@scana scana commented Mar 3, 2022

Adds support for ScaleType.CENTER_CROP flag - previously this would be ignored and would leave some space on either side of the view.

Closes #220

I would appreciate you testing this on your side @Canato, @PiotrBandurski - this helps with our use-case but the chance is that it breakes somebody elses.

@scana scana requested a review from a team as a code owner March 3, 2022 15:38
@Canato
Copy link
Member

Canato commented Mar 3, 2022

@scana please check the CI. Maybe need to run code style

And when you can, please update the CHANGELOG

@scana
Copy link
Contributor Author

scana commented Mar 4, 2022

@Canato done :)

Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

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

Hey @scana doing some tests when I change the ScaleType on the sample app it look strange

Video to explain:

screen-20220304-122141.1.mp4

@scana
Copy link
Contributor Author

scana commented Mar 4, 2022

That's what I meant with our use-case working fine 🙃 - thank you for the video, I'll take a closer look as soon as possible (and test it with the sample app this time)

@scana
Copy link
Contributor Author

scana commented Mar 5, 2022

Hey @Canato just wanted to double-check with you - I've run the sample app off the main branch and got the exact same results you got.

After I switch to my branch/PR however I get this (seems ok 🤔 ):

device-2022-03-05-174947.3.mp4

@Canato
Copy link
Member

Canato commented Mar 7, 2022

Hey @Canato just wanted to double-check with you - I've run the sample app off the main branch and got the exact same results you got.

After I switch to my branch/PR however I get this (seems ok 🤔 ):

oohhh no =|
We may have a bug on Main... will test a little more later today and if is the case merge this PR and open a new bug issue

@Canato Canato merged commit eeb7a99 into CanHub:main Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - ScaleType Problem
2 participants