Skip to content

Fix image cropped vertically#447

Closed
etoledom wants to merge 5 commits intodevelopfrom
issue/444-image-cropped-vertically
Closed

Fix image cropped vertically#447
etoledom wants to merge 5 commits intodevelopfrom
issue/444-image-cropped-vertically

Conversation

@etoledom
Copy link
Copy Markdown
Contributor

@etoledom etoledom commented Dec 25, 2018

Fixes #444

Gutenberg side: WordPress/gutenberg#13096

The image size implementations tries to follow closely the web implementation. I tried to share some code from ImageSize but most of it is platform specific. I made a small utils to share some math calculations.

On a future PR I plan to implement some kind of loading indicator.
cc @iamthomasbishop

image_height

To test:

  • Run the example project.
  • Scroll down to the last block.
  • Check that the image displays correctly as shown in the attached image.

@etoledom etoledom added the [Type] Bug Something isn't working label Dec 25, 2018
@etoledom etoledom added this to the Beta milestone Dec 25, 2018
@etoledom etoledom self-assigned this Dec 25, 2018
@etoledom etoledom requested a review from Tug December 25, 2018 12:38
@etoledom etoledom mentioned this pull request Dec 25, 2018
Merged
@iamthomasbishop
Copy link
Copy Markdown
Contributor

iamthomasbishop commented Dec 25, 2018

Awesome! Thanks for fixing this @etoledom!!

@etoledom
Copy link
Copy Markdown
Contributor Author

etoledom commented Dec 26, 2018

Hey @iamthomasbishop - glad you like it :)

I'm wandering what do you think about an activity indicator when the image is loading?
This is how it looks now:

image_loading

(That is pretty much the behavior on web, but on mobile I would expect some kind of activity indicator)

@iamthomasbishop
Copy link
Copy Markdown
Contributor

iamthomasbishop commented Dec 26, 2018

I would also expect a loading indicator. Is there any chance we can utilize the same approach that we took on Aztec uploads, where you see a loader over the top of the media as it loads?

@etoledom etoledom mentioned this pull request Dec 26, 2018
Closed
@etoledom
Copy link
Copy Markdown
Contributor Author

etoledom commented Dec 26, 2018

I created a new issue for the activity indicator task.
Let's move that conversation to: #451

@etoledom
Copy link
Copy Markdown
Contributor Author

etoledom commented Jan 16, 2019

Hey @Tug !
I updated the gutenberg reference to latest master. Everything is working in my local tests.
Please take a look before the final merge.
Thank you!

@etoledom
Copy link
Copy Markdown
Contributor Author

etoledom commented Jan 18, 2019

The gutenberg reference already was updated in develop so this PR is not needed anymore.

@etoledom etoledom closed this Jan 18, 2019
@etoledom etoledom deleted the issue/444-image-cropped-vertically branch January 28, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Tug Tug Awaiting requested review from Tug

Assignees

@etoledom etoledom

Labels

[Type] Bug Something isn't working

Projects

None yet

Milestone

Beta

Development

Successfully merging this pull request may close these issues.

Image block cropps image vertically.

2 participants

@etoledom @iamthomasbishop