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

Custom CSS Distributed With Dictionaries #542

Closed
MarvNC opened this issue Jan 18, 2024 · 3 comments · Fixed by #1080
Closed

Custom CSS Distributed With Dictionaries #542

MarvNC opened this issue Jan 18, 2024 · 3 comments · Fixed by #1080
Assignees
Labels
kind/enhancement The issue or PR is a new feature or request

Comments

@MarvNC
Copy link
Member

MarvNC commented Jan 18, 2024

LGTM. (Though why does this work like this at all...? Wouldn't it make more sense for a dictionary to distribute some CSS (maybe with basic filtering to avoid remote resources) and then just refer to class names in the structured content...?)

Originally posted by @djahandarie in #527 (review)

I always thought the same, it seemed quite limiting. Maybe the reason is compatibility with light and dark themes and ensuring a consistent experience by limiting what can be styled:

Correct, this is the rationale for not originally adding them. That and they can't be themed easily by custom CSS, which leads to an inconsistent UI/UX. There is probably a better way to support this, such as by having the extension expose custom CSS vars for dictionaries to use for colors, but ultimately that would still just involve using custom string values for color/background-color.

Originally posted by @toasted-nutbread in #450 (comment)

@MarvNC MarvNC added the kind/enhancement The issue or PR is a new feature or request label Jan 18, 2024
@toasted-nutbread
Copy link

CSS scoping is an issue, if dictionaries are basically just injecting a <style> node with rules. If the CSS classes are defined in a way that is more similar to structured content, but having common type definitions for common classes, the issue is a bit different, in that there suddenly needs to be a way to store and retrieve that in the database.

Again the reason why it is currently set up that way is because when I designed it, I did not foresee it growing into such a broadly used feature as it is today.

@djahandarie
Copy link
Collaborator

djahandarie commented Jan 20, 2024

I think we have three options for scoping:

  • Do some preprocessing or verification on the CSS to modify the selectors with some relevant scope, and/or verify that the appropriate scope is already in the selector (i.e., have a parent selector that matches on the dictionary name / revision id, etc etc.)
  • Use a shadow DOM (not sure if it's possible)
  • Use @scope, but no Firefox support yet

@toasted-nutbread
Copy link

toasted-nutbread commented Jan 20, 2024

Shadow dom is possible but probably very ugly. Problems with Anki export though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement The issue or PR is a new feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants