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

Allow unknown file extensions to be selected in file open dialog #6263

Merged

Conversation

rasteiner
Copy link
Contributor

@rasteiner rasteiner commented Feb 12, 2024

This PR …

Resumes the work of #4344, but without merge conflicts.

It:

  • Implements the acceptAttribute method in the FileBlueprint class.
  • deprecates the acceptMime method
  • adds a $matchWildcard attribute to the Kirby\Filesystem\Mime::toTemplates() method.
  • changes the upload field mixin and the files section config to use acceptAttribute instead of acceptMime.

The new acceptAttribute function returns a list of extensions, instead of the list of mime types.

The primary merit is that this allows files to be uploaded which are unknown to Kirby, but without forwading an all allowing * to the browser file open dialog. Because Kirby and the browser no longer need to know a mime type for those files.

The modified Mime::toTemplates method can now produce an array of extensions also for a mime type like image/*.

Breaking changes

I'm not sure if the deprecation is a breaking change, in which case we could just not deprecate the no longer used toMime() method.

Docs

acceptAttribute also has a clearer docblock which mentions how the list of file extensions is actually built:

  • If there is a mime type, the "extension" and "type" options are ignored.
  • If there is no mime type, only the intersection between the "extension" and "type" is returned (not their union).
    The reproduces how acceptMime worked.

Ready?

I think so :)

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@distantnative distantnative added the type: bug 🐛 Is a bug; fixes a bug label Feb 17, 2024
@distantnative distantnative added this to the 4.2.0 milestone Feb 17, 2024
src/Cms/FileBlueprint.php Outdated Show resolved Hide resolved
src/Cms/FileBlueprint.php Outdated Show resolved Hide resolved
src/Cms/FileBlueprint.php Outdated Show resolved Hide resolved
src/Cms/FileBlueprint.php Outdated Show resolved Hide resolved
src/Filesystem/Mime.php Show resolved Hide resolved
tests/Cms/Blueprints/FileBlueprintTest.php Outdated Show resolved Hide resolved
Copy link
Member

@distantnative distantnative left a comment

Choose a reason for hiding this comment

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

Thanks for adding the changes. I think @lukasbestle should have the final review.

Copy link
Member

@lukasbestle lukasbestle left a comment

Choose a reason for hiding this comment

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

I think the logic is awesome as a first step without breaking changes. Thanks! 👍

src/Cms/FileBlueprint.php Show resolved Hide resolved
src/Cms/FileBlueprint.php Outdated Show resolved Hide resolved
tests/Cms/Blueprints/FileBlueprintTest.php Outdated Show resolved Hide resolved
tests/Cms/Blueprints/FileBlueprintTest.php Outdated Show resolved Hide resolved
@bastianallgeier bastianallgeier merged commit 9cff247 into getkirby:develop-minor Mar 7, 2024
7 checks passed
@bastianallgeier
Copy link
Member

I'm really glad we were able to finally merge this. What a marathon :) Thanks for your patience and your contribution, Roman!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants