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

docs: fix overlapping of open in playground button #17403

Merged
merged 3 commits into from Jul 25, 2023

Conversation

Tanujkanti4441
Copy link
Contributor

Prerequisites checklist

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

[x] 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
[ ] Other, please explain:

What changes did you make? (Give an overview)

The open in Playground button is overlapping the code in PC size.

current after this PR
Screenshot 2023-07-22 195236 Screenshot 2023-07-22 195203
Screenshot 2023-07-22 195427 Screenshot 2023-07-22 195452

And also overlapping the back to top arrow button.

Screenshot 2023-07-22 202619

So as a fix some padding is added to the code part.
And added z-index: 1 property to scroll up button.

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

@Tanujkanti4441 Tanujkanti4441 requested a review from a team as a code owner July 22, 2023 15:16
@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Jul 22, 2023
@netlify
Copy link

netlify bot commented Jul 22, 2023

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 9ec26c8
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/64beb54b267df50008e5080e
😎 Deploy Preview https://deploy-preview-17403--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 added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jul 22, 2023
@nzakas
Copy link
Member

nzakas commented Jul 24, 2023

Did you mean to also change this for mobile? It looks like this currently:
Screenshot 2023-07-24 at 11-14-50 no-cond-assign - ESLint - Pluggable JavaScript Linter

Comment on lines 124 to 131
.incorrect,
.correct {
@media all and (min-width: 768px) {
pre.line-numbers-mode {
padding-bottom: 4rem;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we give the above style to all devices? so that I will fix the mobile view look also?

Copy link
Member

Choose a reason for hiding this comment

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

It looks good if we give the bottom padding for the code block as 4rem and the playground button bottom as 1rem

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!
WhatsApp Image 2023-07-24 at 10 57 04 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have kept the bottom position 1.5rem in PC size because it is touching the scrollbar in 1rem.

Screenshot 2023-07-24 231146

@Tanujkanti4441
Copy link
Contributor Author

Did you mean to also change this for mobile? It looks like this currently.

the overlapping issue is just seen in the PC size. so padding is added only for the screen size above or equal to 768px.

@amareshsm
Copy link
Member

Did you mean to also change this for mobile? It looks like this currently.

the overlapping issue is just seen in the PC size. so padding is added only for the screen size above or equal to 768px.

In the mobile view, it is not overlapping but the UI doesn’t look good it looks odd. so we can fix the mobile view UI also.

Copy link
Member

@amareshsm amareshsm left a comment

Choose a reason for hiding this comment

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

LGTM. Waiting for nzakas review.

Copy link
Member

@nzakas nzakas 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!

@nzakas nzakas merged commit 6d6dc51 into eslint:main Jul 25, 2023
22 checks passed
@Tanujkanti4441 Tanujkanti4441 deleted the fix-up-btn branch July 26, 2023 07:48
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jan 22, 2024
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion contributor pool documentation Relates to ESLint's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants