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.

Sign up for GitHub

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

Jump to bottom

docs: add border-radius to hX:target selector styles #19270

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

docs: add border-radius to hX:target selector styles #19270

wants to merge 5 commits into from
+2 −0

Conversation

lumirlumir
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[X] Other, please explain:

What changes did you make? (Give an overview)

Hello,

I've resolved the issue mentioned in eslint/eslint.org#662

What problem do you want to solve?

Hello, I noticed that when clicking the link button next to headings, the background-color changes, as shown in the screenshot below.

Image

However, I found that the border-radius for the hX:target selectors are not set to var(--border-radius);. This causes inconsistency in the visualization, as seen in the following screenshot.

Image

What do you think is the correct solution?

I suggest adding a border-radius: var(--border-radius); property to the hX:target selectors to enhance visual consistency.

Here’s a preview of the changes:

  • Before
    Image
  • After
    Image

Participation

  • I am willing to submit a pull request for this change.

Additional comments

I think adding a bit more top and bottom padding or margin could improve the UI. Just my opinion—feel free to take it into consideration!

Is there anything you'd like reviewers to focus on?

@lumirlumir lumirlumir requested a review from a team as a code owner December 24, 2024 09:36
@eslint-github-bot eslint-github-bot bot added the chore This change is not user-facing label Dec 24, 2024
@netlify Netlify
Copy link

netlify bot commented Dec 24, 2024 •
edited
Loading

✅ Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 21603c2
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/676fff9eb88d11000808cf26
😎 Deploy Preview https://deploy-preview-19270--docs-eslint.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 configuration.

@amareshsm amareshsm changed the title chore: add border-radius to hX:target selector styles docs: add border-radius to hX:target selector styles Dec 24, 2024
@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Dec 24, 2024
@lumirlumir
Copy link
Contributor Author

lumirlumir commented Dec 24, 2024 •
edited
Loading

We can add padding to the h1, h2, h3, h4, h5, h6 selectors, such as padding: 0.25rem 0, as shown in the screenshot below:

image

However, I don't think adding left and right padding is a good idea, as it can make the headings appear indented, as demonstrated in the screenshot below (using padding: 0.25rem):

image

Additionally, padding values cannot be applied to hX:target selectors without causing a layout shift when users click on the heading.

For these reasons, I believe using padding: 0.25rem 0 or padding: 4px 0 (or similar values) to the h1, h2, h3, h4, h5, h6 selectors would be a better approach.

If you have any other thoughts or suggestions, please feel free to share them!

@lumirlumir lumirlumir requested a review from amareshsm December 28, 2024 13:39
@lumirlumir
Copy link
Contributor Author

lumirlumir commented Dec 28, 2024 •
edited
Loading

I've updated it.

Tanujkanti4441
Tanujkanti4441 approved these changes Dec 28, 2024
View reviewed changes
Copy link
Contributor

@Tanujkanti4441 Tanujkanti4441 left a comment

Choose a reason for hiding this comment

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

LGTM! thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tanujkanti4441 Tanujkanti4441 Tanujkanti4441 approved these changes

@amareshsm amareshsm Awaiting requested review from amareshsm

Assignees
No one assigned
Labels
chore This change is not user-facing documentation Relates to ESLint's documentation
Projects
Triage
Status: Second Review Needed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lumirlumir @amareshsm @Tanujkanti4441