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

uppy.io/examples - stop css from affecting uppy #266

Merged
merged 6 commits into from
Jul 22, 2024
Merged

Conversation

lakesare
Copy link
Contributor

@lakesare lakesare commented Jul 16, 2024

Fixes transloadit/uppy#4439

Makes headings normal-sized & fixes heading font issues

Before

image

After

image

Removes double shadow

Before

image

After

image

Increases right-margin for checkboxes

Before

image

After

image

In general, in CSS, changes:
all tag references (e.g. h1),
to unique class references (e.g. .h1_src-pages-examples-module).

Hopefully that makes the "Uppy CSS is broken on examples page" issue appear less frequently.

@lakesare lakesare changed the title Lakesare/examples css uppy.io/examples - stop css from affecting uppy Jul 16, 2024
@lakesare lakesare requested a review from Murderlon July 16, 2024 19:55
@lakesare lakesare marked this pull request as ready for review July 16, 2024 19:55
Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

I don't think this is the correct solution. It should not be possible to override the CSS of Uppy with simple selectors like .main h1. We should build protections into Uppy to prevent this, either with specificity and uppy-reset class as we've been doing or the more ambitious undertaking of using cascade layers.

We can merge this as a quickfix, but it will just make us blind the problems other users are still facing.

@lakesare
Copy link
Contributor Author

lakesare commented Jul 17, 2024

There are 2 solutions in this pr:

  • not targetting <section>s in uppy.io css
    I think we agree that removing .section css rules in uppy.io is the correct solution here.
    Noone targets .sections on a page on production sites; if we were guarding against that we'd need to add .clear to every div in uppy. This would just complicate style overrides.
  • not targetting <h1> in uppy.io css
    Kind of agreed with yours/artur's comment wrt the <h1> part, I was on the brim here.
    It is indeed frequent for people to apply font changes to semantic elements site-wide, and it is a sensible thing to do.

No need for a quickfix merge, I'll revert the "uppy.io/examples - make font alright too" commit in favor of making Uppy more robust against outside css.

@lakesare lakesare requested a review from Murderlon July 17, 2024 19:30
@lakesare
Copy link
Contributor Author

lakesare commented Jul 17, 2024

but it will just make us blind the problems other users are still facing.

I don't think we should treat uppy.io page as our testing ground for "what css issues users might be having".
What happens again and again is our demo css gets fucked up, and then it takes months to get a fix in, because it's just not a high-priority issue.

If we think it's important then we should add h1/etc. styles to dev/Dashboard.js to make sure in development that Uppy defends against that.

So, I think the default behaviour when editing out uppy.io css should be to use high-specificity selectors, e.g. classes, like standardized in this pr (h1 is an exception, because outside framework is bringing that in for us - and because it is indeed good for Uppy to defend against that).

@lakesare lakesare added this pull request to the merge queue Jul 22, 2024
Merged via the queue into main with commit 28722fa Jul 22, 2024
2 checks passed
@lakesare lakesare deleted the lakesare/examples-css branch July 22, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Styles for camera permission screen are off
2 participants