feat(website): redesign of showcase page

[chimailo]

Motivation

This PR was created as a result of this issue #4669 and after this design was approved.

Have you read the Contributing Guidelines on pull requests?

I certainly have and have tried to follow its guidelines.

Test Plan

Dark Theme

Light theme

[slorber]

Hey, thanks

The site can't build in the CI however so I can't check the preview.

It may be a problem with our write-language CLI, but you can probably fix it by renaming either the Tag comp or Tag prop type name.

---

Some initial feedback:

• We probably still want to keep favorites at the top of the list when no tags are selected: those sites must stand out for casual visitors
• Not sure how your UX works exactly, but IMHO we must see tags and description before hovering the cards
• icons can help identify features more easily faster, but it will be hard to scale it to a lot of tags so it's not mandatory to keep?

[netlify]

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: e957dd9

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61965fa62577090007305cab

😎 Browse the preview: https://deploy-preview-5742--docusaurus-2.netlify.app

[github-actions]

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	81
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

Lighthouse ran on https://deploy-preview-5742--docusaurus-2.netlify.app/

[chimailo]

I've changed the design of the card to meet the specifications in the comment above.
For the last point, I think that it is easier to see the tags associated with an icon than having to remember what tag an icon represents.

For the tooltip, is it okay to use a third party library like tippy.js?

[slorber]

Review url: https://deploy-preview-5742--docusaurus-2.netlify.app/showcase/

That looks nice thanks.

Some raw comments:

• Tippy JS is 40.8kB, that's a lot just for tooltips. We need something simple and lightweight (that we could eventually use in other places and add to Infima later?), and I believe using just CSS tooltips might be enough?

• I'd like to keep the Heart icon/emoji to make favorites more visible. The separation between favs and non-favs is not very visible. It's worth adding a separator + label like "all sites" + remove favs from the rest of the list?

• cards are not rendered very consistently. Source buttons should rather remain aligned, and card ratios (screenshot/text) should rather be the same everywhere. We could limit text by clamping to 3 lines + hover to display full-text (if not possible for all browsers, overflow hidden can be fine).

[Josh-Cena]

@chimailo It seems you have been inactive for a while (~two weeks). I'm going to take this over soon after (maybe less than a week?) if it seems you are unavailable to finish this PR😉

[chimailo]

@chimailo It seems you have been inactive for a while (~two weeks). I'm going to take this over soon after (maybe less than a week?) if it seems you are unavailable to finish this PRwink

@Josh-Cena I've actually been working on it, in the time I have, the tooltip component was a little more challenging than I thought.

[Josh-Cena]

Also, merge main so we can see the latest changes applied & be ready for review

[chimailo]

• We need something simple and lightweight (that we could eventually use in other places and add to Infima later?), and I believe using just CSS tooltips might be enough?

I tried using just CSS for the tooltips, but it always had issues, one if which is the positioning of the tooltip. The closer the tooltip is to the edge of the viewport, the more likely it is to for part of it to be cut off from view, hence the need for javascript (I used popper js for this) to handle positioning of the tooltip.

Another is that the parent element could limit/affect the display of the tooptip. Certain css properties like overflow: hidden on the parent element may limit what is displayed to scope of the parent container. Since css can't handle this, I used react portals to detach the tooltip from its parent and place it somewhere else in the DOM tree.

[slorber]

Thanks, that starts to look nice :)

I'm ok for using popperjs, it's a quite generic and widely used lib that we can reuse in other parts later

[chimailo]

except that the filter toggle has no visual cue when focused and also doesn't accept keyboard inputs

The space-bar key toggles the checkbox that is focused, the problem is just that one couldn't tell when it is focused. I've made sure to highlight the filter toggle when it is focused.

[slorber]

I love it, thanks a lot! Didn't expect it to be so beautiful!

Some feedback:

• Should we allow to click on the image or does it cause any trouble?

• What about allowing to click on card tags? Does it lead to a weird UX?

• Should favorites still be at the beginning if tags are used?

• Would it be easy to animate a bit the tooltip?

• Maybe we need a separator, at least for our light theme, because some screenshots have white background?

• I think we should normalize on card ratios, to avoid a weird offset:

(if images are not good enough we can update them, and eventually add a CI check to ensure a required image ratio? => this can be done in another PR)

---

All this is optional, merging this now is already good enough to me!

[chimailo]

• Should we allow to click on the image or does it cause any trouble?

I'm assuming you mean making the card images links? That would make the images not load since the image only loads when it is clicked.

• What about allowing to click on card tags? Does it lead to a weird UX?

I don't think so. At least, it would help those using keyboard navigation.

• Should favorites still be at the beginning if tags are used?

They could be at the beginning but I don't think we need to separate them as they now can be easily identified.

• Would it be easy to animate a bit the tooltip?

For that I would need to use a transition library (React Transition Group). Didn't want to introduce another library, I'd implement it in the next push.

• Maybe we need a separator, at least for our light theme, because some screenshots have white background?

Sure.

• I think we should normalize on card ratios, to avoid a weird offset: (if images are not good enough we can update them, and eventually add a CI check to ensure a required image ratio? => this can be done in another PR)

I think so to. The image ratios vary very broadly and ensuring an image ratio is the best way to solve that.

[Josh-Cena]

• What about allowing to click on card tags? Does it lead to a weird UX?

I thought about that but it's genuinely a bit weird. You click on a tag and then the page's scroll position goes back to the top.

(if images are not good enough we can update them, and eventually add a CI check to ensure a required image ratio?)

No—we should just use CSS. That's too much limitation on the contributor side for little returns.

[slorber]

No—we should just use CSS. That's too much limitation on the contributor side for little returns.

I don't agree here, we should not add a strict ratio but a minimum one. Most users already send good looking screenshots, only some are bad.

This should be rejected by CI, because of not enough height:

We'll fix that in another PR

---

Ok so I suggest we just do some minor changes to this PR merge it:

• add more visible card separator
• replace ref by state
• filter favs ahead of time

[Josh-Cena]

All addressed! 👍 Things look a lot cleaner now. I'll merge this shortly after, can't wait to see it in production

[slorber]

awesome, thanks :)

[slorber]

Just noticed the toggle background looks a bit weird

Also we should rename Facebook to Meta

[Josh-Cena]

Just noticed the toggle background looks a bit weird

Good catch

Also we should rename Facebook to Meta

🌚

[Josh-Cena]

FB has given itself a name so easily misunderstood in a lot of contexts... Docusaurus sites of Meta projects

[slorber]

Yes 🤷‍♂️ 😄 but I think it's required to do the change now

[Josh-Cena]

It's quite late here, so...😉

[lex111]

I also noticed that aria-describedby attribute in every <li> element refers to a non-existent element.

Rocket Validator confirms it:

Also underline links does not work correctly for multiline text:

Maybe you don't need to merge this PR so quickly? Why the rush? This is not the first time I've noticed a PR in the spirit of "Missed this in ..." like that #5965. Probably we need to introduce a rule not to self-merge PR until it get at least one approval from any maintainer? Of course in the case of a tiny/insignificant change a maintainer can self-merge such a PR as before.

[Josh-Cena]

Probably we need to introduce a rule not to self-merge PR until it get at least one approval from any maintainer?

Well, we did get approval in both cases ("Good to merge, just minor changes"). It's quite hard to catch everything especially if the diff, the preview, and CI all look misleadingly good (e.g. #4095). Sometimes it leads to bugs like #5863, other times it's less severe or is caught early. I doubt if any of us would be able to catch this had we been reviewing #4095.

But, good point. I do think up to this point all of us have kept in mind to get some review before merging anything significant.

[slorber]

not a big deal IMHO, it's not bad to merge it now and fix the remaining issues later as it's good enough in current state

It's only impacting our own website, not users, so I'm ok to move fast and break things a bit on such PRs.

We should be more careful in reviews in particular when new public APIs are created

[lex111]

Yes, agreed, but just wanted to remind not to jump to merge a PR. Although at the repository level we could try to require an one approval to be able to merge a PR (just in case).