Adding an optional image service based on Squoosh #4738
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
added 18 commits
September 1, 2022 16:38
🦋 Changeset detectedLatest commit: ff553cb The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: integration
added 6 commits
September 13, 2022 18:10
github-actions
bot
removed
the
pkg: astro
|
!preview wasm |
|
matthewp
approved these changes
Sep 21, 2022
sarah11918
reviewed
Sep 22, 2022
There was a problem hiding this comment.
Looking good, @tony-sull !
Left one comment describing the notation we use for Asides (no blockquotes for things that aren't actually quotations!) but this occurs in six different places. So, whatever you do for one, please do everywhere. 😄
Then, it's good for Docs!
| @@ -195,6 +220,8 @@ A `number` can also be provided, useful when the aspect ratio is calculated at b | |||
| **Default:** `undefined` | |||
| </p> | |||
|
|
|||
| > This is not supported by the default Squoosh service. See the [installation section](#installing-sharp-optional) for details on using the `sharp` service instead. | |||
There was a problem hiding this comment.
Suggested change
| > This is not supported by the default Squoosh service. See the [installation section](#installing-sharp-optional) for details on using the `sharp` service instead. | |
| :::note | |
| This is not supported by the default Squoosh service. See the [installation section](#installing-sharp-optional) for details on using the `sharp` service instead. | |
| ::: |
@delucis doesn't let me abuse blockquote anymore... 😅
This is the format for asides that we use in Docs. If you want a custom title (instead of just note), then you can use the format
:::note[custom title that will be in all caps]
text
:::
If this looks too weird on the GitHub README directly, there's always GitHubs own new-ish "Note" syntax:
Note
This is a note
Whatever you do here, should be done in all six instances of this (3 properties of <Image />, 3 properties of <Picture />)
There was a problem hiding this comment.
How does :::note work in NPM? I'm wondering if we eventually want a README that goes to NPM and a separate DOCS.md that is hosted on the docs site 🤔
There was a problem hiding this comment.
We eventually do want most of this info to live in Docs... 😅 Let me see if I can find an example of one of our Asides that is currently in a README, so we can check to be sure what it looks like...
There was a problem hiding this comment.
The very top of this file uses:
> ⚠️ This integration is still experimental! Only node environments are supported currently, stay tuned for Deno support in the future!
So, I guess there's precedent for that for now. We don't need to hold this up over the Aside/Blockquotes. And, I just discovered that GitHub's own Note syntax just renders as a blockquote in our Docs anyway, with no special display at all...
So, maybe for this case, let's just keep the blockquote or use GitHub's own notation which at least looks nicer here, (both will look like blockquote on the docs, so up to you) and we'll figure out a better long-term solution.
tony-sull
changed the title
|
!preview wasm |
|
1 task
Merged
1 task
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.
Live Demo on StackBlitz
Changes
Testing
sharpservice. The wasm service is noticeably slower in CI runs and can run into memory issues if multiple suites are all fired up at once, each loading their own copies of the.wasmlibrariesDocs
README updated with details on the new wasm-based service and notes for
<Image />props that are only supported bysharp