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

Accidental blur effect on BLUR reference page #650

Open
wong-justin opened this issue Dec 2, 2024 · 3 comments · May be fixed by #664
Open

Accidental blur effect on BLUR reference page #650

wong-justin opened this issue Dec 2, 2024 · 3 comments · May be fixed by #664
Labels
Bug Something isn't working Help Wanted Extra attention is needed

Comments

@wong-justin
Copy link

wong-justin commented Dec 2, 2024 •
edited
Loading

Most appropriate sections of the p5.js website?

Reference

What is your operating system?

Windows 10

Web browser and version

Chromium v122.0, Firefox v133.0

Actual Behavior

When visiting https://p5js.org/reference/p5/BLUR/, the whole page is blurred because of a .blur class applied to the <html> tag.

image

Expected Behavior

The page should be unblurred like all the other pages at p5js.org:

image

Steps to reproduce

  1. Visit https://p5js.org/reference/p5/BLUR/ in any web browser
  2. Observe blurriness

Would you like to work on the issue?

Maybe?
@wong-justin wong-justin added the Bug Something isn't working label Dec 2, 2024
@wong-justin
Copy link
Author

wong-justin commented Dec 2, 2024 •
edited
Loading

Each page on the site has a class name applied here:

p5.js-website/src/layouts/BaseLayout.astro

Lines 65 to 68 in fe363ad

<html
class={`${titleClass.toLowerCase()} ${className} scroll-smooth`}
lang={currentLocale}
>

Normally this doesn't produce any conflicts, like how the /about page has custom styles defined in a .about class. But on the blur page, this conflicts with the preexisting tailwind .blur class.

Maybe on this page, the class name could be renamed to blur_ or something similar?

Also, I'm not sure if there's any other naming conflicts on other reference pages.

@davepagurek
Copy link
Collaborator

davepagurek commented Dec 2, 2024 •
edited
Loading

It's probably a good idea overall to do some namespacing to these title classes since they've caused some issues in the past too. Maybe in ReferenceLayout we can do something like:

titleClass={`reference_${title.toLowerCase()}`}

...so we have .reference_blur? I don't think we have any page-specific class names for individual reference pages yet.

@ksen0
Copy link
Contributor

ksen0 commented Dec 6, 2024 •
edited
Loading

Hello @wong-justin ! Great catch. The above suggestion also makes a lot of sense to me, that the classes in the reference should all have a prefix, like reference_${title.toLowerCase()}, to avoid namespace collision everywhere, not only in this instance. What do you think / do you want to work on this and create a PR?

@ksen0 ksen0 added the Help Wanted Extra attention is needed label Dec 10, 2024
wong-justin added a commit to wong-justin/p5.js-website that referenced this issue Dec 19, 2024
@wong-justin
give class names a prefix on reference pages
84a45f1 
fixes processing#650
before: blur reference page would have .blur CSS style applied
after: blur reference page has .reference_blur class name,
so there are no more collisions with CSS styles

hopefully this doesn't have any unintended side effects
@wong-justin wong-justin linked a pull request Dec 19, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Something isn't working Help Wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Give class names a prefix on reference pages wong-justin/p5.js-website
3 participants
@ksen0 @davepagurek @wong-justin