feat: Add HTML class customization settings#85
feat: Add HTML class customization settings#85justinmayer merged 6 commits intopelican-plugins:mainfrom tstreule:patch-1
Conversation
Introduce `IMAGE_PROCESS_REMOVE_CLASS` setting, allowing the removal of `image-process-<transform>` CSS classes after image processing. These classes are user-assigned to trigger transformations, but once processed, they may no longer be needed. If enabled, the class is removed, and if it was the only class, the `class` attribute is deleted entirely.
justinmayer
left a comment
justinmayer
left a comment
There was a problem hiding this comment.
If we are going to add a new setting, I think my inclination would be to add a IMAGE_PROCESS_ADD_CLASS setting instead.
And instead of a Boolean, maybe make it a string prefix that defaults to the current image-process-<transform> pattern. That way the class name could be customized to the user's liking, or the class addition could be omitted entirely by configuring the setting to any "falsy" value.
What does everything think about this alternative approach?
|
@justinmayer I feel we should have two separate settings for what you are proposing: one to define the |
|
If you like, I can adjust the code by adding a |
- Renamed `IMAGE_PROCESS_REMOVE_CLASS` to `IMAGE_PROCESS_ADD_CLASS` (default: `True`) - Introduced `IMAGE_PROCESS_CLASS_PREFIX` (default: `"image-process-"`) for class name customization - Updated documentation to reflect these changes
|
It looks okay to me. Generally, I am hesitant to add too many configuration settings, but I can see the use case. Thank you as well for updating the documentation! Ideally, there should be a test for this (to make sure it works as expected as doesn't get broken in future updates). If it's trivial to add, go ahead and add it to this PR. Otherwise, we can merge this PR and let's raise an issue to track adding the tests. |
|
@minchinweb I added some tests for the new settings. I also added a |
|
I have been traveling but will review this in the next day or two. It shouldn’t be merged in its current form, if only because the release file will need modifications to incorporate the other changes that have been merged since the last release. |
justinmayer
changed the title
IMAGE_PROCESS_REMOVE_CLASS setting
justinmayer
left a comment
justinmayer
left a comment
There was a problem hiding this comment.
Many thanks to @tstreule for the enhancement and to @patrickfournier and @minchinweb for reviewing! 🏅
As a very minor suggestion, in the future it would be better to put the unrelated README whitespace/formatting changes in a separate commit and pull request. That makes it easier when reviewing the commit history, using the Git blame feature, etc. But please don't let this minor suggestion detract from the excellent work here, which is greatly appreciated! 😁
justinmayer
changed the title
IMAGE_PROCESS_REMOVE_CLASS setting4 participants
Introduce the settings
IMAGE_PROCESS_ADD_CLASSandIMAGE_PROCESS_CLASS_PREFIX, allowing customization of the image's CSS classimage-process-<transform>.IMAGE_PROCESS_ADD_CLASScontrols whether theimage-process-<transform>CSS class is added to images after processing. By default, this setting is enabled (True). If disabled, no class is added to the image.IMAGE_PROCESS_CLASS_PREFIXallows customization of the class prefix. The default prefix isimage-process-, but this can be changed to a custom value.