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

Plugin asset CSS files miss timestamp in URL #5164

Closed
stairjoke opened this issue Apr 30, 2023 · 3 comments · Fixed by #5641
Closed

Plugin asset CSS files miss timestamp in URL #5164

stairjoke opened this issue Apr 30, 2023 · 3 comments · Fixed by #5641
Assignees
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Milestone

Comments

@stairjoke
Copy link

Description

According to Plugin Basics > Timestamps any asset included inside a plugin folder should automatically receive a timestamp when cached in the media folder. This is not happening, and it seems this has not worked for several years:

Expected behavior
Referring to an asset included inside a plugin should return a URL that points to a cached file which has a URL such as this:
/media/plugins/superwoman/superplugin/myCss.1520265293.css

Actual Behavior
The actual result of referring to an asset inside a plugin folder is:
/media/plugins/superwoman/superplugin/myCss.css

Screenshots
CleanShot 2023-04-30 at 11 35 23@2x

CleanShot 2023-04-30 at 11 34 45@2x

To reproduce

  1. Go to create a plugin and add a CSS file inside the asset folder of your plugin
  2. Include the CSS file in a page template using css('PATH')
  3. Open a page using this template
  4. Look for the <link>-tag in the HTML

Your setup

Kirby v3.9.1

Console output

Your system (please complete the following information)

  • PHP: 8.1.17

Additional context

@stairjoke stairjoke changed the title Plugin asset CSS files are miss timestamp in URL Plugin asset CSS files miss timestamp in URL May 4, 2023
@rasteiner
Copy link
Contributor

related: getkirby/getkirby.com#148

@distantnative
Copy link
Member

@lukasbestle I think even with our changes #5247 this remains too complicated for developers. I think we need to rewrite it once more to allow something like

$plugin->asset('foo.js')->url();
// -> URL to media folder with timestamp included (based on modified)

// maybe also more methods
$plugin->asset('foo.js')->exists();
$plugin->asset('foo.js')->publish();
$plugin->asset('foo.js')->unpublish();
$plugin->asset('foo.js')->path();
$plugin->asset('foo.js')->root();

Unfortunately, I think Filesystem\Asset is currently very much tied to the assets folder. Ideally, we'd make that class more flexible as well and reuse here.

@lukasbestle
Copy link
Member

@distantnative I like that syntax. However I wouldn't add ->exists(), IMO the asset() method should still return null if the asset does not exist. Otherwise it would be really hard to find bugs because the asset URL would still be generated and one would have to search for reasons why the URL doesn't work. If it returns null, there's immediately an error.

@distantnative distantnative linked a pull request Sep 16, 2023 that will close this issue
5 tasks
@distantnative distantnative self-assigned this Oct 8, 2023
@distantnative distantnative added this to the 4.0.0-beta.3 milestone Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants