Skip to content

Commit

Permalink
SvgImage: Bundle spinner.gif properly so it can be resolved with webp…
Browse files Browse the repository at this point in the history
…ack, etc. (#273)

## Summary:

**UPDATE**: I kept running into issue after issue with packaging a `.gif`. Finally, I decided to try a different approach. We have a WB spinner, so why not just use that? 

We have had a longstanding issue with the `SvgImage` preloader. It references an image (`spinner.gif`) using an absolute path. Even worse, we don't bundle that image with Perseus, expecting this image to live at `/images/spinner.gif` in whatever environment Perseus is used. 

This PR changes it so that:
  * we now bundle the spinner.gif with Perseus
  * we import it using a relative path import (which makes it so webpack, or whatever web build tool you use, can process the image and bundle it however it needs)

Issue: LP-11765

## Test plan:

Check out the Storybook story: **Components** >> **SVG Image** >> **Svg Image That Doesn't Load**

<img width="500" alt="image" src="https://user-images.githubusercontent.com/77138/175366272-4ff57c66-a28b-428d-b25c-5c35a8f74466.png">
Note: The spinner does not appear in the right place. This appears to be a long-standing, pre-existing bug that I will fix/work on in a separate PR.

Author: jeremywiebe

Reviewers: jaredly, jeresig, jeremywiebe, kevinbarabash

Required Reviewers:

Approved By: jaredly, jeresig

Checks: ✅ codecov/project, ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset file (ubuntu-latest, 16.x)

Pull Request URL: #273
  • Loading branch information
jeremywiebe committed Jun 24, 2022
1 parent c48c68a commit 988726b
Show file tree
Hide file tree
Showing 11 changed files with 331 additions and 39 deletions.
5 changes: 5 additions & 0 deletions .changeset/shy-flowers-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Change default "preloader" for `SvgImage` to use a WonderBlocks spinner. This avoids the need to deal with a bundled spinner GIF entirely.
11 changes: 11 additions & 0 deletions packages/perseus/src/components/__stories__/svg-image.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ export const MostlyEmptyPropsObject = (args: StoryArgs): React.Node => {
return <SvgImage alt="ALT" />;
};

export const SvgImageThatDoesntLoad = (args: StoryArgs): React.Node => {
return (
<SvgImage
alt="ALT"
height={100}
width={500}
src={"http://httpstat.us/200?sleep=1000000"}
/>
);
};

export const SvgImageBasic = (args: StoryArgs): React.Node => {
return <SvgImage src={svgUrl} alt="ALT" />;
};
Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/components/fixed-to-responsive.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ const FixedToResponsive: $FlowFixMe = createReactClass({
maxHeight: height,
};

// NOTE(jeremy): This depends on styles defined in perseus-renderer.less
const className = classNames(
"fixed-to-responsive",
this.props.className,
Expand Down
17 changes: 10 additions & 7 deletions packages/perseus/src/components/svg-image.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable no-useless-escape, react/no-unsafe */
// @flow
import {CircularSpinner} from "@khanacademy/wonder-blocks-progress-spinner";
import classNames from "classnames";
import $ from "jquery";
import * as React from "react";
Expand Down Expand Up @@ -119,14 +120,19 @@ function defaultPreloader(dimensions: Dimensions) {
return (
<span
style={{
background: "url(/images/spinner.gif) no-repeat",
backgroundPosition: "center",
top: 0,
left: 0,
width: "100%",
height: "100%",
position: "absolute",
minWidth: "20px",
display: "flex",
justifyContent: "center",
alignContent: "center",
}}
/>
>
<CircularSpinner size="medium" />
</span>
);
}

Expand Down Expand Up @@ -591,10 +597,7 @@ class SvgImage extends React.Component<Props, State> {
const width = this.props.width && this.props.width * this.props.scale;
const height =
this.props.height && this.props.height * this.props.scale;
const dimensions = {
width: width,
height: height,
};
const dimensions = {width, height};

// To make an image responsive, we need to know what its width and
// height are in advance (before inserting it into the DOM) so that we
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,25 @@ exports[`categorizer widget should snapshot on mobile: first mobile render 1`] =
style="padding-bottom: 101.36%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -407,8 +424,25 @@ exports[`categorizer widget should snapshot on mobile: first mobile render 1`] =
style="padding-bottom: 91.39%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
</div>
Expand Down Expand Up @@ -884,8 +918,25 @@ exports[`categorizer widget should snapshot: first render 1`] = `
style="padding-bottom: 101.36%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -916,8 +967,25 @@ exports[`categorizer widget should snapshot: first render 1`] = `
style="padding-bottom: 91.39%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1489,8 +1489,25 @@ exports[`grapher widget should snapshot question with multiple graph types: init
style="padding-bottom: 100%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
<div
class="graphie-container"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,25 @@ exports[`group widget should snapshot: initial render 1`] = `
style="padding-bottom: 112.14999999999999%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -1023,8 +1040,25 @@ exports[`group widget should snapshot: initial render 1`] = `
style="padding-bottom: 21.05%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
<span
class="perseus-sr-only"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,25 @@ exports[`image widget - isMobile %b should snapshot: first render 1`] = `
style="padding-bottom: 82.14%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
<span
class="perseus-sr-only"
Expand Down Expand Up @@ -246,8 +263,25 @@ exports[`image widget - isMobile %b should snapshot: first render 2`] = `
style="padding-bottom: 82.14%;"
/>
<span
style="background: url(/images/spinner.gif) no-repeat; background-position: center; width: 100%; height: 100%; position: absolute; min-width: 20px;"
/>
style="top: 0px; left: 0px; width: 100%; height: 100%; position: absolute; min-width: 20px; display: flex; justify-content: center; align-content: center;"
>
<div
class="default_xu2jcg-o_O-spinnerContainer_agrn11"
>
<svg
height="48"
viewBox="0 0 48 48"
width="48"
xmlns="http://www.w3.org/2000/svg"
>
<path
class="loadingSpinner_1dvgeb3-o_O-inlineStyles_1kzk0y9"
d="M44.19 23.455a1.91 1.91 0 1 1 3.801 0h.003c.004.18.006.363.006.545 0 13.255-10.745 24-24 24S0 37.255 0 24 10.745 0 24 0c.182 0 .364.002.545.006V.01a1.91 1.91 0 1 1 0 3.801v.015A20.564 20.564 0 0 0 24 3.818C12.854 3.818 3.818 12.854 3.818 24c0 11.146 9.036 20.182 20.182 20.182 11.146 0 20.182-9.036 20.182-20.182 0-.182-.003-.364-.007-.545h.015z"
fill-rule="nonzero"
/>
</svg>
</div>
</span>
</div>
<span
class="perseus-sr-only"
Expand Down

0 comments on commit 988726b

Please sign in to comment.