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

Lemmy doesn't display light themes correctly #570

Closed
kujaw opened this issue Mar 1, 2020 · 10 comments
Closed

Lemmy doesn't display light themes correctly #570

kujaw opened this issue Mar 1, 2020 · 10 comments

Comments

@kujaw
Copy link

kujaw commented Mar 1, 2020

On Palemoon 28.8.3, comments and body text is vrey bright, making it unreadable on light background.
On Firefox 73.0.1 it displays correctly.
Images and conversation is in the link: https://dev.lemmy.ml/post/30957

@kujaw kujaw changed the title Lemmy doesn't display light themes correctly [Bug] Lemmy doesn't display light themes correctly Mar 1, 2020
@kujaw kujaw changed the title [Bug] Lemmy doesn't display light themes correctly Lemmy doesn't display light themes correctly Mar 1, 2020
@dessalines
Copy link
Member

Lemmy initially loads darkly, then unloads it, and replaces it with whatever theme you have. The fact that it works fine on FF, Qutebrowser, Chrome, even android browsers, should tell you that this is an issue with palemoon, not lemmy.

Here's the code if the palemoon devs are interested:

export function setTheme(theme: string = 'darkly') {
  // unload all the other themes
  for (var i = 0; i < themes.length; i++) {
    let styleSheet = document.getElementById(themes[i]);
    if (styleSheet) {
      styleSheet.setAttribute('disabled', 'disabled');
    }
  }

  // Load the theme dynamically
  if (!document.getElementById(theme)) {
    var head = document.getElementsByTagName('head')[0];
    var link = document.createElement('link');
    link.id = theme;
    link.rel = 'stylesheet';
    link.type = 'text/css';
    link.href = `/static/assets/css/themes/${theme}.min.css`;
    link.media = 'all';
    head.appendChild(link);
  }
  document.getElementById(theme).removeAttribute('disabled');
}

@wolfbeast
Copy link

wolfbeast commented Mar 2, 2020

I'm sorry but the disabled attribute on <link> is not specced in the HTML standard. There has been some discussion about it because apparently it's the only way in Chrome/Blink to disable style sheets (of which the logic is really fishy (not my words)) but since it's not in the spec to support the "disabled" attribute, we don't. As a result the darkly theme remains loaded and is overridden by other themes in proper cascading style fashion, except, of course, where darkly has an !important attribute set so it breaks the cascade. this is the case for color on darkly's body selector, overruling the loaded light themes which don't use !important.

Please be more browser-agnostic in the way you switch themes. I suggest avoiding any and all !important rules in your default theme so styles can be applied dynamically on top, or not loading darkly by default before applying the chosen theme.

@dessalines
Copy link
Member

Palemoon is the odd one out here, looks like all browsers support this.

Here's mozillas spec: https://developer.mozilla.org/en-US/docs/Web/API/StyleSheet/disabled

@wolfbeast
Copy link

MDN documentation is not a spec.
And this will apply to all UXP-based browsers. Pale Moon ,Basilisk, IceWeasel, Hyperbola's suite, Borealis when it's finally released, etc.

Ultimately it's your decision of course to use something that's not standard. All I can do is ask that you please don't -- which I now have.

@dessalines
Copy link
Member

dessalines commented Mar 2, 2020

@EchedelleLR
Copy link

The draft: https://drafts.csswg.org/cssom/#dom-stylesheet-disabled

You know that is a recent draft right?, so you are using features that are not yet even considered standard just because main known engines (chrome, edge and opera are the same) implemented it??

!important

Using stylesheets with that specification in every declaration is misrecommended at professional level and even learning level. it is common when you have a lot of css and have to deal with a lot of legacy css code mostly but this is not a wordpress child theme, this is just Lemmy...

@dessalines
Copy link
Member

If you have another way to load a different theme on the page, without refreshing it, and without using css disabled, I'm all ears.

@EchedelleLR
Copy link

EchedelleLR commented Aug 14, 2020

Idk about Lemmy source code but would be Invidious way to change theme valid for the use case?

They have just a toggle switch calling a function which saves the information in the localStorage (i think is for maintaining it after reloading if the user do it) to change the theme in the same moment without reloading and css disabled.

When reloading the info seems to be saved in a not displayed span but that is out of scope about my knowledge.

@EchedelleLR
Copy link

@dessalines
Copy link
Member

PRs welcome.

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

No branches or pull requests

4 participants