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

Unsplash image block #143

Merged
merged 34 commits into from
Jun 5, 2020
Merged

Unsplash image block #143

merged 34 commits into from
Jun 5, 2020

Conversation

ravichdev
Copy link
Contributor

Summary

Fixes #127 #128 #131

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Contributing Guidelines (updates are often made to the guidelines, check it out periodically).

php/class-plugin.php Outdated Show resolved Hide resolved
php/class-plugin.php Outdated Show resolved Hide resolved
php/class-plugin.php Outdated Show resolved Hide resolved
@spacedmonkey
Copy link
Contributor

Are translation files being load correctly in the block editor?

await page.waitForSelector( '.wp-block-unsplash-image' );

// Image is inserted.
expect( await page.$$( '.wp-block-unsplash-image' ) ).toHaveLength( 1 );
Copy link
Contributor

Choose a reason for hiding this comment

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

This has the same issue I had with my e2e tests, as it times out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the test to import a smaller size image ~500kb, let's see if it resolves travis timeout issue

remove_filter( 'wp_get_attachment_url', [ $this->plugin->hotlink, 'wp_get_attachment_url' ], 10 );
remove_filter( 'image_downsize', [ $this->plugin->hotlink, 'image_downsize' ], 10 );

$image = wp_get_attachment_image_src( $id, $attributes['sizeSlug'] );
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if sizeSlug are not set for some reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

or set to full?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added code to check for sizeSlug and set it to full if it does not exist. But this value would always be set in the block editor.

@ravichdev ravichdev changed the title WIP: Unsplash image block Unsplash image block Jun 3, 2020
@spacedmonkey
Copy link
Contributor

Code cover is down 3%.

return createBlock( 'core/image', attributes );
},
},
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be able to transform to a img block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that's what we were discussing last week ? The block should be able to transform into the core image block.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, as in, add a transform from img to an unsplash. Leave the current one there.

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

Amazing work!

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

Changing view, as did not work with gutenberg plugin

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

Approved!

@ravichdev ravichdev merged commit b9d17eb into develop Jun 5, 2020
@ravichdev ravichdev deleted the feature/unsplash-image-block branch June 5, 2020 10:35
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.

A user is presented with a custom block called "Unsplash"
2 participants