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

feat(www): tagline on plugin library landing page #12530

Merged
merged 18 commits into from
Mar 21, 2019
Merged

Conversation

calcsam
Copy link
Contributor

@calcsam calcsam commented Mar 12, 2019

This is an idea for a tagline for the plugin page to illustrate how versatile the Gatsby plugin ecosystem is.

What do y'all think? @KyleAMathews @fk @marcysutton @amberleyromo?

If we want to ship this we should:

  • turn it off (set to "anything") if there's anything in the search bar to avoid it being distracting
  • see if we want to pretty up the design at all (tweak the line spacing on 3rd line, hover on the links)

Screen Recording 2019-03-12 at 03 43 PM

@calcsam calcsam requested a review from a team as a code owner March 12, 2019 22:43
@calcsam
Copy link
Contributor Author

calcsam commented Mar 12, 2019

Alternate design, a bit more lightweight (second commit). Instead of < and > we probably want slightly larger, clickable chevron icons that overlap both lines. But you get the idea.

Screen Recording 2019-03-12 at 03 58 PM

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

This looks great! It looks a bit janky to me so I'm thinking of making the widths of the dynamic parts fixed so you get some extra spacing if it's smaller but it won't jump around. Another option which takes a bit more time is to use translates to move the text in front. I can do that in a followup.

The second one is more subtle as it has only 1 moving part but unsure if it's clear enough. I'll let the pros decide 😄

I love the design, it makes it more interactive!

})
}
componentDidMount() {
const intervalId = setInterval(() => this.incrementItem(), 5000);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should work with onAnimationEnd event instead of an interval? If we keep an interval I would suggest to use setTimeout instead as setInterval might not be 5s over time and setTimeout always will be.

@marcysutton
Copy link
Contributor

One thing I'm wondering is if there's a way to introduce a stop button so the rotating text doesn't distract people. We could use prefers-reduced-motion in CSS or JS to respect user preferences in supported browsers...but an additional icon button would help, too.

@calcsam
Copy link
Contributor Author

calcsam commented Mar 13, 2019

these seem like good ideas! right now clicking the arrows will stop the rotation.

@fk offered to take this over to prettify this and get it merged in!

@fk
Copy link
Contributor

fk commented Mar 13, 2019

@wardpeet

This looks great! It looks a bit janky to me so I'm thinking of making the widths of the dynamic parts fixed so you get some extra spacing if it's smaller but it won't jump around. Another option which takes a bit more time is to use translates to move the text in front. I can do that in a followup.

💯, https://github.com/braposo/react-text-loop/tree/master/ -like behavior would be great.
To avoid the extra spacing, maybe we could tweak the copy to something that allows us to put an equivalent to the current "Need" on its own line … e.g.

Looking for
responsive images?

or something?

💯 to everything that @marcysutton said, too — maybe if we don't want to invest in play/pause just hide the whole thing if the user prefers-reduced-motion?

@wardpeet
Copy link
Contributor

We could go for react-text-loop to get this one going. I'm not the biggest fan as it's quite big https://bundlephobia.com/result?p=react-text-loop@2.0.1
image 20kb is pretty big in js world

@marcysutton
Copy link
Contributor

Whatever we do end up using, I'd love to review it for accessibility prior to shipping as rotating things usually need a little help.

@fk
Copy link
Contributor

fk commented Mar 19, 2019

First measures to get an initial version ready to ship:

  • added icons and screenreader text ("Previous", "Next") to the control buttons, turned them into actual <button>s (so they are focusable), and updated their position:

    Instead of < and > we probably want slightly larger, clickable chevron icons that overlap both lines

  • added animation of the alternating text's width

dee04465-88d7-40c0-b271-04cb5fb32b27

@fk
Copy link
Contributor

fk commented Mar 19, 2019

Simpler:

aac1e22d-5cd2-40c6-812f-e2e456bcebb3

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

@fk awesome job on this. Left some nit-picky React comments (which I'm happy to make the changes!) but otherwise this looks so nice.

www/src/components/rotater.js Outdated Show resolved Hide resolved
www/src/components/rotater.js Outdated Show resolved Hide resolved
www/src/components/rotater.js Outdated Show resolved Hide resolved
www/src/components/rotater.js Outdated Show resolved Hide resolved
www/src/components/rotater.js Outdated Show resolved Hide resolved
www/src/components/rotater.js Outdated Show resolved Hide resolved
@fk
Copy link
Contributor

fk commented Mar 20, 2019

Wooah thanks @DSchau for the review! Right in time for @wardpeet and me pairing to push this over the finish line. Btw. what's your favorite — minimal or "welled"?

@DSchau
Copy link
Contributor

DSchau commented Mar 20, 2019

@fk I like the minimal--presuming that's the one without a background and a slight grey border?

But they both look great 💜

@fk
Copy link
Contributor

fk commented Mar 20, 2019

"minimal" is this one:

aac1e22d-5cd2-40c6-812f-e2e456bcebb3

And you are in favor of that one Edit: No you are not, I just can't read anymore 😅 — this is "welled":

dee04465-88d7-40c0-b271-04cb5fb32b27

@DSchau
Copy link
Contributor

DSchau commented Mar 20, 2019

Just to be 100% clear I like this one better :) But I defer to you--both are great!

Image

<Slider
items={[text]}
color={`#000`}
getSize={size => this.setState({ size })}
Copy link
Contributor

Choose a reason for hiding this comment

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

Argh! Thanks for catching that @wardpeet! 🙏

@fk
Copy link
Contributor

fk commented Mar 20, 2019

@wardpeet and myself (with the help of @DSchau 🙏) settled on the "minimal" variant, and think it's good to ship as a first version:

Summing up, we modified @calcsam's first shot to

  • animate the alternating text's width to make the transition less "jumpy"
  • play nice on smaller screens (in context of the current slider content)
  • update control buttons UI and position according to @calcsam's comment

    Instead of < and > we probably want slightly larger, clickable chevron icons that overlap both lines

  • hopefully be accessible enough to ship — @marcysutton could you please take a look and let us know if we missed anything?
    • turn control buttons into actual <button>s (so they are focusable), add icons, screenreader text ("Previous", "Next"), and aria-controls attributes
    • honor prefers-reduced-motion — if set, we completely disable fade and width animations
    • aria-live set to either off or polite, depending on prefers-reduced-motion

We didn't look into disabling the slider if there's anything in the search bar because we didn't find it too distracting.

/cc @calcsam

@calcsam
Copy link
Contributor Author

calcsam commented Mar 20, 2019

Holy smokes, this looks beautiful @fk!

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

This does indeed look beautiful.

Nicely done everyone! Pulling it down and taking a look; will merge in a bit!

@DSchau DSchau changed the title feat(docs): tagline on plugin library landing page feat(www): tagline on plugin library landing page Mar 21, 2019
@sidharthachatterjee
Copy link
Contributor

sidharthachatterjee commented Mar 21, 2019

Just leaving this to mention that this looks so beautiful! Love it. Great work Flo!

@DSchau DSchau merged commit d9c11ca into master Mar 21, 2019
@DSchau DSchau deleted the tagline-on-plugins-page branch March 21, 2019 15:37
@DSchau
Copy link
Contributor

DSchau commented Mar 21, 2019

Heads up - this broke SSR because we weren't guarding window with a window.matchMedia call (easy to miss!). Will PR a quick fix in a sec!

pieh pushed a commit that referenced this pull request Mar 21, 2019
## Description

Fixes a tiny regression re: SSR rendering introduced in #12530

Validated this by:

1. Ensuring rotater animates (by default!) in built version
1. Ensuring rotater _does not_ animate when reduced motion is enabled in System Preferences - Accessibility
1. Ensuring site can build with `gatsby build` & `gatsby serve`
@fk
Copy link
Contributor

fk commented Mar 21, 2019

🙏 @DSchau! (and @sid, and @calcsam, and @marcysutton, and @wardpeet!)

@wardpeet
Copy link
Contributor

Ugh my bad sorry 🙏

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.

7 participants