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

Add a File field, like for .pdf uploads #74

Merged
merged 18 commits into from
Jul 20, 2021
Merged

Add a File field, like for .pdf uploads #74

merged 18 commits into from
Jul 20, 2021

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Jun 14, 2021

Changes

  • Add a File field that allows non-image files:

ioabsdifadsf

  • There's already an Image field, but that doesn't allow some files like .pdf files

Fixes GF-3071

Testing instructions

  1. cd /path/to/wp-content/plugins/genesis-custom-blocks/

  2. git fetch

  3. git checkout add/file-field

  4. composer install && npm i && npm run build

  5. /wp-admin > Custom Blocks > Add New

  6. Add a field with the type 'File':

file-here

  1. Click Publish

  2. /wp-admin > Posts > Add New

  3. Enter the name of the block you just created:

thebisdf

  1. In the block, click 'Upload' and upload any .pdf file

  2. Expected: The file name displays:

ioabsdifadsf

  1. This won't preview the actual file in the Edit component, as the file could be a .pdf, a .zip, or any number of files

package-lock.json Outdated Show resolved Hide resolved
This should be a bit easier,
it was strange having $image_id.
@kienstra kienstra marked this pull request as ready for review July 9, 2021 01:26
kienstra added 2 commits July 8, 2021 20:34
There were only conflicts in tests/e2e/specs/all-fields.js

They were trival, I kept both.
Align it horizontally with the
help text.
@@ -72,7 +72,7 @@ public function init() {
*/
public function register_hooks() {
add_action( 'enqueue_block_editor_assets', [ $this, 'editor_assets' ] );
add_filter( 'block_categories', [ $this, 'register_categories' ] );
add_filter( 'block_categories_all', [ $this, 'register_categories' ] );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will need to go out before WP 5.8 to prevent the PHP notice.

if ( newImage?.alt ) {
setImageAlt( newImage.alt );
} else if ( newImage?.source_url ) { // eslint-disable-line camelcase
if ( media?.alt ) {
Copy link
Contributor Author

@kienstra kienstra Jul 9, 2021

Choose a reason for hiding this comment

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

Most of this file's diff is just renaming parameters.

Like here, it changes newImage to media, as this hook is now used for files and images.

@johnstonphilip
Copy link

Nice work Ryan! This is working well for me when logged in as an administrator. However, if I log in as a contributor, I see the file field, but it doesn't actually upload the file. This isn't a must-fix right now, but possibly something to add in the future, if the user doesn't actually have permission to upload files. Perhaps we shouldn't show the upload field to them.

Screen Shot 2021-07-19 at 3 07 16 PM

The main thing I wanted to be sure of was user capabilities work, which they do.

I was reading that you need to wrap MediaUpload in MediaUploadCheck here:
https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/media-upload/README.md

@kienstra
Copy link
Contributor Author

Thanks, @dreamwhisper!

Great idea to test it with a contributor, never though of that. Also, good idea to use a <MediaUploadCheck>

kienstra added 2 commits July 19, 2021 16:34
As Jen mentioned,
this can prevent the <MediaUpload> from showing
for users that can't upload files.
There was a trivial conflict in Loader.php.
This isn't the right PR to do it in,
but I forgot in #85.
@kienstra
Copy link
Contributor Author

kienstra commented Jul 20, 2021

If it's OK, I bumped the WP Tested Up To version to 5.8, forgot to do that in #85

@kienstra kienstra merged commit a2aedc4 into develop Jul 20, 2021
@kienstra kienstra deleted the add/file-field branch July 20, 2021 06:08
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.

3 participants