Skip to content

refactor(website): various fixes and improvements on Showcase page #5997

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

Merged
merged 5 commits into from
Nov 26, 2021

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Nov 23, 2021

Motivation

  • Fix HTML errors:
    • The “aria-describedby” attribute must point to an element in the same document.
    • Element “h2” not allowed as child of element “span” in this context. (Suppressing further errors from this subtree.)
  • Fix underline of link (in case of multi-line text)
  • Remove redundant code (styles/attributes). Side-note: aria-describedby should be used on interactive/focusable elements
  • Use outline for focus state (for better a11y and consistency)
  • Preserve scroll position after change filters for nicer UX

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Preview

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@lex111 lex111 added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Nov 23, 2021
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 23, 2021
@netlify
Copy link

netlify bot commented Nov 23, 2021

✔️ [V2]

🔨 Explore the source changes: 3ac587e

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

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

@github-actions
Copy link

github-actions bot commented Nov 23, 2021

⚡️ Lighthouse report for the changes in this PR:

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

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

@github-actions
Copy link

github-actions bot commented Nov 23, 2021

Size Change: -804 B (0%)

Total Size: 891 kB

Filename Size Change
website/build/assets/css/styles.********.css 99.5 kB -809 B (-1%)
ℹ️ View Unchanged
Filename Size Change
website/.docusaurus/globalData.json 38.3 kB 0 B
website/build/assets/js/main.********.js 477 kB 0 B
website/build/blog/2017/12/14/introducing-docusaurus/index.html 66.2 kB 0 B
website/build/blog/index.html 37.8 kB 0 B
website/build/docs/index.html 44 kB +4 B (0%)
website/build/docs/installation/index.html 51.7 kB +1 B (0%)
website/build/index.html 29.5 kB 0 B
website/build/tests/docs/index.html 25.2 kB 0 B
website/build/tests/docs/standalone/index.html 21.7 kB 0 B

compressed-size-action

@Josh-Cena Josh-Cena added pr: documentation This PR works on the website or other text documents in the repo. and removed pr: maintenance This PR does not produce any behavior differences to end users when upgrading. labels Nov 23, 2021
Copy link
Collaborator

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Some stuff (especially the link hover underline) I've noticed before but either don't have the bandwidth / don't know how to fix, so it's great that we figured it out

.showcaseCardTitle a:focus-visible::after {
width: 100%;
.showcaseCardTitle a:not(:focus):hover {
background-size: 100% 1px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL🧐

history.push({
...location,
search: searchParams.toString(),
state: {scroll: window.scrollY},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to preserve the focus here, e.g. refocus to the last toggled element? It's already a great improvement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it makes sense, just made a basic implementation of it, but feel free to improve the code if you want (I won't have a chance to do it until the weekend).

@Josh-Cena Josh-Cena changed the title refactor(website): add various fixes and improvements on Showcase page refactor(website): various fixes and improvements on Showcase page Nov 24, 2021

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Copy link
Collaborator

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

LGTM

@Josh-Cena Josh-Cena merged commit d25bf24 into main Nov 26, 2021
@Josh-Cena Josh-Cena deleted the lex111/showcase-imp branch November 26, 2021 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: documentation This PR works on the website or other text documents in the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants