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: place the playground button in the bottom right corner #17848

Closed
wants to merge 5 commits into from

Conversation

kecrily
Copy link
Member

@kecrily kecrily commented Dec 13, 2023

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)

Removes large white space from the code block.

Before After
image image

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

@kecrily kecrily requested a review from a team as a code owner December 13, 2023 15:35
@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Dec 13, 2023
Copy link

netlify bot commented Dec 13, 2023

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit b5a5d51
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/657c7c8f5cf72c00070f5bb6
😎 Deploy Preview https://deploy-preview-17848--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.

@nzakas
Copy link
Member

nzakas commented Dec 13, 2023

It does look a bit better this way, but I think we'll need to adjust the styles in light mode:

Screenshot 2023-12-13 at 10-56-14 getter-return - ESLint - Pluggable JavaScript Linter

I think the button needs a shadow or border to help make the edges clearer

@kecrily
Copy link
Member Author

kecrily commented Dec 13, 2023

Is that okay?

image

@fasttime
Copy link
Member

I can see that after this change there will be extra empty space after all code blocks, even if there is no Playground button:

Now After
image image

Is this intended?

@mdjermanovic
Copy link
Member

IMO, it looks better when the button is in the code block. Also, if the button is over the border, when the horizontal scroll bar appears (usually it does on small screens, e.g. mobile), the button will overlap with it:

image

@kecrily
Copy link
Member Author

kecrily commented Dec 15, 2023

Is this intended?

@fasttime Yes, to avoid overlapping with other content on a small screen. But it shouldn't be found in a normal code block. now adjusted.

IMO, it looks better when the button is in the code block. Also, if the button is over the border, when the horizontal scroll bar appears (usually it does on small screens, e.g. mobile), the button will overlap with it:

@mdjermanovic I fixed it. The scrollbar now no longer overlaps the buttons.

截屏2023-12-16 00 23 18

@Tanujkanti4441
Copy link
Contributor

Screenshot 2023-12-17 222423

in my browser i have different scrollbar is it looking fine?

@kecrily
Copy link
Member Author

kecrily commented Dec 17, 2023

in my browser i have different scrollbar is it looking fine?

There's nothing wrong with its position, but it doesn't look very good. Do I need to adjust these scrollbars to macOS-like thin scroll bars?


Sorry for accidentally closed.

@kecrily kecrily closed this Dec 17, 2023
@kecrily kecrily reopened this Dec 17, 2023
@harish-sethuraman
Copy link
Member

I feel like something like this on hover would be better with help text open in playground probably?
Screenshot 2023-12-18 at 4 30 51 PM

we could use something like this to denote correct and in correct instead?

Screenshot 2023-12-18 at 4 32 02 PM

end output would be somewhere close to this? (😅 edited entire thing in inspect tools design could be better).
Why I'd prefer something like this is because,

  • we show a link to playground when we hover on code sample so it doesn't obstruct the view?
  • the correct in correct doesn't float anymore and lives more with the code block itself
Screenshot 2023-12-18 at 5 23 09 PM

@Tanujkanti4441
Copy link
Contributor

i like the last design and instead of keeping that playground button in the code block we can place it to the correct/incorrect block with the playground name.

@nzakas
Copy link
Member

nzakas commented Dec 21, 2023

This PR was just to move the button -- there's no good reason to change the overall design of the correct/incorrect code examples. If we can't agree on moving the button, then we should just close this PR.

@Tanujkanti4441
Copy link
Contributor

if we shift the button to the bottom right corner we will end up with the same overlapping problem which we had previously

Screenshot 2023-12-23 201015

@amareshsm
Copy link
Member

if we shift the button to the bottom right corner we will end up with the same overlapping problem which we had previously

Screenshot 2023-12-23 201015

Yes we need to consider. Boz of this issue we decided to place the button inside the block in #17403

@kecrily
Copy link
Member Author

kecrily commented Dec 23, 2023

Back to top button is floating. It does not interfere with reading, just slide up or down. And even if don't move the button to the bottom right corner, it still overlaps the correct/incorrect icon.

image

@Tanujkanti4441
Copy link
Contributor

Back to top button is floating. It does not interfere with reading, just slide up or down. And even if don't move the button to the bottom right corner, it still overlaps the correct/incorrect icon.
image

comment, as per this comment it is okay if button overlaps the things which can't be clicked.

@kecrily
Copy link
Member Author

kecrily commented Dec 24, 2023

image

Is this OK?

@nzakas
Copy link
Member

nzakas commented Jan 4, 2024

Folks - this discussion has been going on for a while, so my vote is just to leave the button as-is. If it was an easy fix, that would be one thing, but we're continuing to run into edge cases and I just don't think it's worth the effort to continue down this path.

Thoughts?

@harish-sethuraman
Copy link
Member

I'd also vote on keeping the same layout 👍🏻

@nzakas
Copy link
Member

nzakas commented Jan 5, 2024

All right, let's just keep what we have. I appreciate the suggestions and work, but let's move on to more interesting problems. :)

@nzakas nzakas closed this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Relates to ESLint's documentation
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

None yet

7 participants