-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
File Block: Add support for embedding PDFs #30857
Conversation
Size Change: +1.05 kB (0%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
I think this linting failure is incorrect: it's presumably meant to check for IE11 compatibility, but the JS runs correctly in IE11 when I test the functionality. |
On further investigation, the linting failure was actually a build failure. It took about a day of diving down infinite rabbit holes to fix it in f57ef25. 🙃 |
This is a really cool feature! Nice investigation on the newly added capability to build frontend files for blocks. It needs some more work and first-class support in WordPress core as the next step. We need a few more use cases in Gutenberg to ensure the API is useful because there might be interesting use cases for the frontend. |
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.
Cool work here @pento! I've left some comment but nothing major.
I added a few comments to the issue, and received feedback from a few people. Here is the newest design suggestion. That multiple people would find helpful. #19077 (comment) |
I'm marking this PR as needing some follow-up work in WordPress core because of the introduction of the PHP file for the block. We will have to add a note in https://core.trac.wordpress.org/ticket/52991 once it lands. I assume it goes in WordPress 5.8 😄 |
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.
This is going good @pento ! I retested and I've left just a comment. Thanks!
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
4c49af7
to
7bee495
Compare
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.
Great work! 💯 - let's 🚢
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
@@ -34,6 +34,13 @@ | |||
"type": "string", | |||
"source": "html", | |||
"selector": "a[download]" | |||
}, | |||
"displayPreview": { |
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.
I like this option and I believe we should probably extend it to various file types. Like QuickView for Gutenberg.
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, it looks like previews for different file types are being discussed 👍🏻
Description
Replaces #24233.
See #19077.
This PR adds an option to the File block, when a PDF file is inserted, to show an embedded version of the PDF.
PDFs are a bit of a special case amongst files: they're the only document file type with native embed support in most major browsers (mobile devices excepted).
How has this been tested?
The following scenarios need to be tested:
Screenshots
Screen.Recording.2021-04-15.at.4.10.34.pm.mov
Types of changes
New feature.
Checklist: