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

Image block vol 03 (sizing) #147

Closed
wants to merge 11 commits into from
Closed

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Sep 17, 2018

Part of #140

This PR implement the changes related to Image Block sizing on the Gutenberg project.

Please follow the instruction on the mentioned PR for testing.

image_block_resizing

@etoledom etoledom mentioned this pull request Sep 17, 2018
10 tasks
@etoledom
Copy link
Contributor Author

@hypest - Could you please take a look at this? Thank you!

I'll check the merge conflict first time tomorrow. My first test didn't work well. (Updating the gutenberg reference from master seems to breaks the build 😩 )

@etoledom
Copy link
Contributor Author

Sent an update and finally everything seems to be green in both projects! 🎉
Thanks @daniloercoli for fixing the build error!

I believe this is ready for a review.

Copy link
Contributor

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

@@ -35,10 +35,6 @@ registerBlockType( UnsupportedBlock.name, UnsupportedBlock.settings );
setUnknownTypeHandlerName( UnsupportedBlock.name );

const initialHtml = `
<!-- wp:image -->
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there is some background info here on why we remove the "upload placeholder" case. Can you elaborate @etoledom ? Thanks!

Copy link
Contributor Author

@etoledom etoledom Sep 18, 2018

Choose a reason for hiding this comment

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

For organization.

I added both together before for testing purposes, but I had in mind to leave just one from the beginning. Now we can add the empty one by adding a new Image Block.

There's no problem to add the empty one back if we want to have everything visible anyway! 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. No, that's cool, no need to reinstate it. It was just not clear because the actual code changes are on the GB repo side and I wasn't sure if we lost that feature or not. Thanks!

@etoledom
Copy link
Contributor Author

This PR has become irrelevant. We can push a new one when we retake the Images sizing work.

@etoledom etoledom closed this Nov 21, 2018
@etoledom etoledom deleted the feature/image-block-vol-03 branch November 21, 2018 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants