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

Image filters #1322

Merged
merged 5 commits into from Aug 27, 2018
Merged

Image filters #1322

merged 5 commits into from Aug 27, 2018

Conversation

lukaiser
Copy link
Contributor

Hey guys

Instead of adding the SVG support right away, as I was asking in #1218, I added 4 filters instead:

  • pb_is_valid_image_extension
  • pb_is_valid_image_type
  • pb_epub201_fetchAndSaveUniqueImage_filename
  • pb_epub201_fetchAndSaveUniqueImage_compress

They allow a plugin developer to add other images to the epub export or use other image generators then wordpress.com.

An example for a SVG plugin:
https://github.com/eSkript/pressbooks-epub-with-svg

I hope that is a solution that makes everybody happy.

Lukas

@codecov
Copy link

codecov bot commented Aug 13, 2018

Codecov Report

Merging #1322 into dev will decrease coverage by <.01%.
The diff coverage is 68.18%.

@@             Coverage Diff              @@
##                dev    #1322      +/-   ##
============================================
- Coverage     61.25%   61.25%   -0.01%     
- Complexity     4127     4129       +2     
============================================
  Files           112      112              
  Lines         18079    18084       +5     
============================================
+ Hits          11075    11078       +3     
- Misses         7004     7006       +2

\Pressbooks\Image\resize_down( $format, $tmp_file );
} catch ( \Exception $e ) {
return '';
/**
Copy link
Contributor

@dac514 dac514 Aug 22, 2018

Choose a reason for hiding this comment

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

Can you move this filter into themeOptionsOverrides?

Set $this->compressImages = false next to this code:

https://github.com/pressbooks/pressbooks/blob/dev/inc/modules/export/epub/class-epub201.php#L403

So that it will be easier to spot what's going on if ever there's an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I see your point, but then it isn't the same filter anymore. How I implemented the filter, you can turn of compression on a per image base and that is needed here, as I don't want to turn of compression for all the images.

@greatislander greatislander merged commit 86cd726 into pressbooks:dev Aug 27, 2018
@greatislander
Copy link
Contributor

Thanks @lukaiser! After some further discussion, we're fine with this as is.

@greatislander greatislander mentioned this pull request Aug 27, 2018
@lukaiser
Copy link
Contributor Author

@greatislander thanks guys!

@lukaiser lukaiser deleted the image-hooks branch August 28, 2018 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants