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

Make spinner visual treatment work better in various sizes #154

Merged
merged 2 commits into from
Jul 26, 2021

Conversation

lyzadanger
Copy link
Contributor

This PR replaces the thin-line "tail-spin" circle SVG image used in the spinner pattern and Spinner component with a "spoked-star" animated SVG that renders more clearly in various sizes. The updated image is closer to the visual style of the client's spinner, whereas the former image was taken from LMS.

This PR also takes the opportunity to convert the pattern-library pages related to spinners over to use the newer Library components.

Because handling of icon-naming and -registration is still up in the air, this PR does not attempt to alleviate any of that directly.

Note: The following images are animated GIFS. They aren't the best for capturing the smoothness of animations, and can make animation speed or transitions look a bit janky. They're also blurry, which is annoying.

Before these changes:

old-spinners

After these changes:

new-spinners

Background

The initial design of the spinner pattern and the Spinner component in this project was based on the local implementation in the lms project: an animated SVG using an image from https://github.com/SamHerbert/SVG-Loaders

The decision to use lms' spinner design versus the client's was one of implementation. In the PR for adding the spinner pattern and Spinner component, I mentioned in a comment that I prefer the visual styling of the client spinner, but the SVG implementation from LMS:

While I'm still not 100% in love with the exact visual styling of the animated SVG per our own design style (p.s. I did take a look at other options in the originating SVG repo and this seemed to be one of the best alternatives). There's nothing wrong with the SVG! I just think the visual style of the (complexly-styled) version in the client is a little more in line with our design language.

Problems with the first SVG loading image

I'm revisiting this now because the tail-spin LMS-style animated SVG turns out to be nearly invisible when rendered at smaller sizes. I noticed this when I started experimenting with replacing client spinners with the component from this package. And it's often rendered on light grey, which makes it even harder to see.

None of the other SVG images from the https://github.com/SamHerbert/SVG-Loader project (nor several other open-source sources I perused) seemed to do well at 1em square.

I was able to find a 12-spoke star-like loading SVG out there in the wild. It had two shortcomings: most importantly, it was of unclear licensing/provenance, but the 12 spokes were too many; it looked tight and weird at certain sizes. What I realized when looking at the source of that image was that I could easily just roll my own with math and svg elements.

I devised an 8-spoke animated loading SVG that I feel works solidly at the sizes we need. It is similar in visual style to the spinner used in the client today (that one is implemented via relatively complex CSS and nested span elements so couldn't be directly reproduced in an SVG).

The place the Spinner from this project's package is used is in the VitalSource book-picker flow in LMS. I think the new version looks good:

updated-vs-spinner

(In case anyone is worried, I added a manual several-second delay there to demonstrate this visual spinner).

Consistency and moving forward

While the spoked SVG loader looks good in the VitalSource book picker (IMO), you might have realized that this leaves us with inconsistency in the loading spinners in LMS. If the spoked variant is amenable, I'd like to move forward with momentum to get all spinners in LMS converted to the shared (spoked) variant after this lands.

One of the things we'll need for LMS is a "Full-screen spinner", which renders a centered spinner on a full-page overlay. I've already started developing this on a local branch. Ideally, I'd like to get the updated spinner treatment and the full-screen spinner patterns and components included in the next release of the package, so that we're ready to update spinners across LMS should we choose.

Update SVG image for spinner to one that renders more comfortably in
a variety of sizes.
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

LGTM. Given the repetition in the SVG I wondered whether it might make sense to generate the spokes in JS, but using a static SVG has the advantage that we can use it in server-rendered HTML.

I don't have particularly strong opinions about the style of the loader, but I agree we should cater to a variety of sizes. It might end up looking better to use different designs for very large spinners than very small ones.

@lyzadanger
Copy link
Contributor Author

I don't have particularly strong opinions about the style of the loader, but I agree we should cater to a variety of sizes. It might end up looking better to use different designs for very large spinners than very small ones.

Yup! I agree! And that's how we "got here" at this point, based on real-world usage. You might note that I retained the "tail-spin" animated icon in this project for now (spinner--circle.png), as I anticipate we may, possibly, "want it back" for, say, a variant, or if the spoke style causes uproar on LMS. We're still flexible!

@lyzadanger lyzadanger merged commit 20f9a46 into main Jul 26, 2021
@lyzadanger lyzadanger deleted the update-spinner-design branch July 26, 2021 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants