-
Notifications
You must be signed in to change notification settings - Fork 21
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
Replace ssr with image card edit component #398
Conversation
0361a37
to
4928c49
Compare
- Requires updating test due to error with ResizeObserver
Pull Request Test Coverage Report for Build 2835022400Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @PatelUtkarsh ! I left a few comments but none of them are blockers, I'll leave this as approved and you can either address them or merge if you think we can go on without them
@@ -245,6 +245,7 @@ public function enqueue_block_editor_assets() { | |||
'postTypes' => $post_types, | |||
'doesRequireBackCompatList' => version_compare( get_bloginfo( 'version' ), '5.8', '<' ), | |||
'canUseQueryLoop' => version_compare( '5.8', get_bloginfo( 'version' ), '<=' ), | |||
'fallBackImageCard' => $this->plugin->asset_url( 'assets/images/placeholder-image-card.png' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥜 Nitpick: I'd prefer if we transform this image to either webp
or at the very least jpg
. I don't see any reason for this to be png and they are usually much larger than they need to be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes generally PNG files are large, but this placeholder image is 4kb - which I've compressed down to 2kb.
@emeaguiar This requires re-review, Can you check? |
Summary
Fixes #331
Checklist