Skip to content

Conversation

@tobimori
Copy link
Contributor

@tobimori tobimori commented Jan 5, 2023

This PR …

Adds asset methods that work similar to file methods or page methods, just for use with the Kirby/Filesystem/Asset class that's e.g. used by the asset() helper.

Why is this important?

There are many plugins out there that can e.g. apply image transforms such as johannschopplich/kirby-blurry-placeholder or my own kirby-blurhash that one might want to use on a static image, but they only work conveniently with Kirby files itself due to file methods not being supported for simple assets.

See also: johannschopplich/kirby-blurry-placeholder#15

This should also be in favor of Kirby, as the docs state: "Anything in your public path can be converted to an Asset object to use the same handy file methods as for any other Kirby files."
They obviously can't be allowed access to regular custom file methods, as those might access properties only available on the File object.

What else to consider?

An alternative to this could be to allow imageMethods which extend the Kirby/Image/Image class. They would automatically inherit to assets and image files, but thus also allow less customization due to the lack of access to e.g.media path info.

Breaking changes

None.

Ready?

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

For review team

@tobimori tobimori changed the base branch from main to develop January 5, 2023 21:59
@tobimori
Copy link
Contributor Author

hey, any update on this PR? is this something you generally don't want, or have you just not found the time to look at it yet?

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.

Sorry for the delay, we were indeed quite busy with 3.9 and 3.9.1. We haven't forgot about your other PR either, but it's going to take a bit of time to review.

I really like the idea of asset methods. I'd say they fit in quite well conceptually. Image methods would have an overlap with the existing file methods, but asset methods are really separate and useful on their own.

lukasbestle
lukasbestle previously approved these changes Jan 26, 2023
@lukasbestle lukasbestle requested a review from a team January 26, 2023 20:56
@lukasbestle lukasbestle requested a review from a team January 26, 2023 20:58
Copy link
Member

@afbora afbora left a comment

Choose a reason for hiding this comment

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

Loved new methods 💛

@afbora afbora requested a review from a team January 27, 2023 09:54
@afbora afbora added this to the 3.9.2 milestone Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants