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

Fix a bug with the image preview bounds #1233

Merged
merged 2 commits into from
Apr 16, 2023

Conversation

luah5
Copy link
Member

@luah5 luah5 commented Apr 13, 2023

Description

this PR fixes a bug where the image preview bounds were the size of the image itself.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

shot.2.mov
shot.3.mov

@austincondiff
Copy link
Collaborator

The image should default to its natural size if that natural size fits within the window. From your screen recording it looks scaled up to fit the container by default. This should only happen if the natural image size is larger than the viewport bounds.

@luah5
Copy link
Member Author

luah5 commented Apr 13, 2023

Good catch! I'll fix that.

@luah5 luah5 changed the title fix bug with the images bounds Fix a bug with the image preview bounds Apr 14, 2023
@austincondiff
Copy link
Collaborator

Can you update your screen recording?

@luah5
Copy link
Member Author

luah5 commented Apr 14, 2023

Sorry I'm seeing this late, anyways, the video has been updated @austincondiff.

@austincondiff
Copy link
Collaborator

It looks like there is a strange thing happening around the rubber band effect. It is like the cap is the current size so when you zoom, it tries to bounce back but it ends up sticking to the larger size so you need to start the gesture over to get it bigger. This results in excessive gestures to get it to the desired size. Is that what I am seeing?

@luah5
Copy link
Member Author

luah5 commented Apr 14, 2023

It's intended behavior @austincondiff, even Preview.app does it when zooming in/out abnormally fast (zooming in/out at a more normal speed does not have this rubberbanding)

shot.4.mov

if you want this changed or removed I can fix this w/

.scrollBounceBehavior(.basedOnSize)

also, should the scroll indicators be hidden?

@austincondiff
Copy link
Collaborator

austincondiff commented Apr 14, 2023

Rubberbanding itself was never in question. We should definitely have rubberbanding. It just looked like from the screen recording that it wasn't allowing you to resize past a certain point in a single gesture. I might be wrong though.

Edit: I am noticing the same in your screen recording of Preview so it may be a false alarm. I think you are just doing many short gestures which let me to my observation.

@luah5
Copy link
Member Author

luah5 commented Apr 14, 2023

I agree w/ your edit; this is me zooming in/out slowly, there is no resizing problem (that was probably misjudged because i have a small trackpad @austincondiff)

shot.5.mov

Copy link
Collaborator

@austincondiff austincondiff 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!

@lukepistrol lukepistrol merged commit 321e265 into CodeEditApp:main Apr 16, 2023
@luah5 luah5 deleted the fix-image-bounds branch April 17, 2023 06:10
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.

🐞 Image preview bounds are image size itself instead of available space
3 participants