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

chore: upgrade Infima to alpha.39 #7306

Merged
merged 7 commits into from
May 4, 2022
Merged

chore: upgrade Infima to alpha.39 #7306

merged 7 commits into from
May 4, 2022

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented May 4, 2022

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Many small changes, also slightly adjusted colors for Prism theme, and collapse/expand button bg made less contrast for dark mode.

Test Plan

Preview

@lex111 lex111 requested review from slorber and Josh-Cena as code owners May 4, 2022 09:59
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 4, 2022
@Josh-Cena Josh-Cena added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label May 4, 2022
@github-actions
Copy link

github-actions bot commented May 4, 2022

Size Change: -2.3 kB (0%)

Total Size: 804 kB

Filename Size Change
website/build/assets/css/styles.********.css 105 kB -2.38 kB (-2%)
website/build/assets/js/main.********.js 610 kB +30 B (0%)
website/build/index.html 38.8 kB +46 B (0%)
ℹ️ View Unchanged
Filename Size
website/.docusaurus/globalData.json 50.7 kB

compressed-size-action

@Josh-Cena
Copy link
Collaborator

CSS is reordered. @slorber I think we should only keep one selector from Infima?

@netlify
Copy link

netlify bot commented May 4, 2022

[V2]

Name Link
🔨 Latest commit 184edd6
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/6272785dc73ba4000bae36de
😎 Deploy Preview https://deploy-preview-7306--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented May 4, 2022

⚡️ Lighthouse report for the changes in this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 59 🟢 100 🟢 92 🟢 100 🟢 90 View report
/docs/ 🟠 67 🟢 94 🟢 92 🟢 90 🟢 90 View report

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.

2.2kB reduction is pretty impressive 👍

If we have the purge CSS plugin, we can probably optimize it further

@Josh-Cena
Copy link
Collaborator

Oh, haha, the h5 issue is long-standing. We should probably lower the accessibility requirement for now. WDYT?

@Josh-Cena
Copy link
Collaborator

This looks cool 😀

@lex111
Copy link
Contributor Author

lex111 commented May 4, 2022

WDYT?

Yes, that's why I created PR #7152 to fix this

This looks cool

I like it too, I think we can shorten first column by using just the path as URL text.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented May 4, 2022

shorten first column by using just the path as URL text.

Agree. Want to do that right now? Ideally, we can use / and /docs/ as the text, and the full URL as the link. (Edited the lighthouse comment to illustrate what I mean)

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

👍 LGTM

Anything left to change?

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

👍 LGTM

Anything left to change?

@slorber
Copy link
Collaborator

slorber commented May 4, 2022

👍 agree the lighthouse check is nice and using pathname is better

what about also adding blog list + blog entry here? If it's not expensive it's worth checking multiple pages

@lex111
Copy link
Contributor Author

lex111 commented May 4, 2022

Agree. Want to do that right now?

Yes, that's what I meant, let's do it in a separate PR.

This PR is ready to merge, I guess.

@slorber
Copy link
Collaborator

slorber commented May 4, 2022

CSS is reordered. @slorber I think we should only keep one selector from Infima?

I guess it's fine to keep it for now. We see when unexpected order changes happen inside Infima, even if we can safely ignore those

@slorber slorber merged commit be0dc6b into main May 4, 2022
@slorber slorber deleted the lex111/infima-upd-april22 branch May 4, 2022 14:06
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: polish This PR adds a very minor behavior improvement that users will enjoy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants