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

fix(theme-classic): add outline to focused code blocks #6121

Merged
merged 2 commits into from Dec 18, 2021

Conversation

christopherklint97
Copy link
Contributor

Motivation

Fixes #6077

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Run yarn start:website and go to http://localhost:3000/docs/next/installation#scaffold-project-website. When focusing on the code block, it should like below:

Firefox
Screenshot 2021-12-17 114011

Chrome
Screenshot 2021-12-17 114258

@netlify
Copy link

netlify bot commented Dec 17, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 267caa1

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

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

@github-actions
Copy link

⚡️ Lighthouse report for the changes in this PR:

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

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

@Josh-Cena
Copy link
Collaborator

The original issue was reproducible on Firefox where pre tags aren't highlighted when focused. Leaving to @slorber to test since he uses FF

@Josh-Cena Josh-Cena added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label Dec 17, 2021
@slorber
Copy link
Collaborator

slorber commented Dec 17, 2021

I can confirm outline on FF works better now.

Comparing:

I can't see any issue so far, not sure why I added this overflow hidden anymore 🤷‍♂️

@@ -9,7 +9,6 @@
margin-bottom: var(--ifm-leading);
border-radius: var(--ifm-global-radius);
box-shadow: var(--ifm-global-shadow-lw);
overflow: hidden;
Copy link
Collaborator

Choose a reason for hiding this comment

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

apparently I added this in #5215, don't remember why 😅 hope it's safe to remove

Copy link
Collaborator

@Josh-Cena Josh-Cena Dec 18, 2021

Choose a reason for hiding this comment

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

I realized what overflow: hidden does... Because if we set border radius on the parent container but child is still a regular rectangle, the child overflows and makes the code block still appear as a sharp-edged rectangle. The reason that the highlight doesn't appear on FF is also because they treat the highlight as part of overflow and gets hidden

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yes, I remember that was related to border-radius now 😅 it was simpler to add radiuses on the container than on the individual items, particularly when the live codeblock playground/result positions are configurable

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Dec 17, 2021

Current production:

image

Deploy preview:

image

Even on Chrome there's an improvement 👀

@Josh-Cena Josh-Cena merged commit 8f18cbb into facebook:main Dec 18, 2021
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.

Use CSS to outline focused code blocks
4 participants