docs(gatsby-plugin-sharp): Base64 features #24999
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gatsbot
bot
added
the
status: triage needed
wardpeet
added
status: needs core review
7 tasks
wardpeet
suggested changes
Jul 3, 2020
wardpeet
changed the title
wardpeet
previously approved these changes
Jul 3, 2020
wardpeet
added
bot: merge on green
|
Seems like tests are failing cc @wardpeet @polarathene |
wardpeet
added
status: awaiting author response
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This feature was added when an internal fixed value of `20` was in use. It was configurable via graphql query parameters as well as overriding the default value in the plugin config, but this was not obvious as no default was set. As the default is `20`, it has been relocated to the plugin defaults, and the redundant method to get the `base64Width` value has been removed(this was implemented before the plugin defaults and options were refactored out to a separate file).
Using a default of `false` is a bit misleading as the expected values are `jpg`,`png` and `webp`, not `true`. The empty string is a falsey value, serving the same purpose. This matches the defaults for similar properties in general args. Additionally updates the docs to show the actual default values and missing options for plugin config options, as these lacked public documentation.
Documents these two parameters for graphql (and their global default override option equivalents via plugin config).
`args` is a parameter, it should not have it's props modified, referencing the `options` object instead where `toFormat` may be modified. Rename the `forceBase64Format` variable to `changedBase64Format`. Should be less likely to imply a boolean, and isn't necessarily the same value as `forceBase64Format` stored on the `pluginOptions`.
polarathene
force-pushed
the
docs/gatsby-sharp-base64-features
branch
from
79ea95a
to
364bbe5
Compare
This comment has been minimized.
This comment has been minimized.
polarathene
removed
the
status: awaiting author response
`healOptions()` needed an update to pull in the `base64Width` default as fallback. base64 tests also needed modifications to avoid caching from calling `base64()` directly instead of via `fixed()` and `fluid()` methods which would invalidate cache and set `width` prop to `base64Width`.
|
All tests should be passing, "Cloud Tests" seems to be hanging for some reason. |
Important for transparent images like PNG when converted to base64 image format like JPG where transparency is not supported. Test is fairly basic relying on snapshot but should be sufficient to indicate if something goes wrong. Could possibly benefit with tests on fluid/fixed where the actual bug was from them not passing the option down.
|
Added a fix for Included a basic test , although like the base64 tests I've added I'm not sure if they're sufficient, should I also be testing these via fluid/fixed tests instead of directly on base64? The bug for I'm not sure if I can mock the base64 method called in that test file for a single test that ensures supported options are passed through? |
|
@wardpeet Could you review/merge please? |
muescha
reviewed
Aug 9, 2020
muescha
reviewed
Aug 9, 2020
wardpeet
reviewed
Aug 10, 2020
@wardpeet where is that question? |
wardpeet
reviewed
Aug 10, 2020
wardpeet
approved these changes
Aug 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Documents some base64 features that Gatsby has supported for a while now. These weren't easily discoverable or clear how to use them.
I did some minor refactoring related to these features. The relevant individual commits provide details in their commit messages.
Documentation
The graphql parameters
toFormatBase64andbase64Widthare now documented in thegatsby-plugin-sharpREADME. The global override equivalent options are mentioned for each, but not documented elsewhere.@gatsbyjs/learning is there any additional documentation of these that would be appreciated? A code snippet or PR to a project that demonstrates the minor features?
Improvements
Some code refactoring, fixes and tests added.
Also
backgroundoption is now passed tobase64(), fixes #25640