Sepitus-exe / WKElementaryDark

A user css theme for the wanikani domain
MIT License
3 stars 1 forks source link

Fix input element with self-study script #18

Closed wrex closed 1 year ago

wrex commented 1 year ago

When the self-study quiz input bar has white text on a white background:

Screenshot 2023-01-14 at 1 13 11 PM

(the entered text is selected so you can see it, otherwise it's invisible).

For reference, here is how it appears with Breeze Dark:

Screenshot 2023-01-14 at 1 17 00 PM

Aside: I wish that WK had a CSS style guide for user scripts. Would make theming a whole lot easier!

Sepitus-exe commented 1 year ago

Looks like the bulk of the issue is the usage of generic selectors e.g. a, span, input[type=''text"]. Making the selectors more strict would somewhat solve the issue, but not provide any inherent formatting for the script. If possible I would like to find a solution that would at least provide color matching for radicals, kanji and vocab.


For now, I have pushed out a temporary fix for this specific plugin, but further work should be done on this.

image

wrex commented 1 year ago

I'll take a look, but I've not yet looked at actually fixing any other userscripts yet (I just opened the issue so it will be tracked).

I'm still trying to understand all the existing rules for the default pages, and attempting to simplify some things if at all possible.

I am making headway (I've mostly finished the dashboard and the review/lessons summary pages already). I've cleaned up a few things and it should be easier to know what will change if you fiddle with any of the custom properties.

One example, I've created the following:

:root {
/* A bunch of custom props for palette colors */

/* Semantic properties */
--ED-text-darkbg: var(--ED-gray-14); /* Light text on a dark background */
--ED-text-lightbg: var(--ED-gray-1); /* Dark text on a light background */

--ED-text-color: var(--ED-text-darkbg); /* Default is light text on a dark background */

This way everything just uses color: --ED-text-color; everywhere. The parent element of anything that sets a light background, though, should redefine --ED-text-color.

For example, the kotoba-table-list now renders like this:

Screenshot 2023-01-14 at 10 18 27 PM

The CSS looks like this:

 :root {
    /* Palette colors -- you _can_ change these, but probably shouldn't */
    --ED-gray-1: #151515; /* darkest */
    /* ... */
    --ED-gray-14: #ffffff; /* lightest */
    --ED-red-1: #9d4745;
    --ED-blue-1: #56638a;
    --ED-green-1: #58b09c;
    --ED-yellow-1: #d7af70;

    /* Semantic definitions. Feel free to change any of these, using the palette colors above */

    /*  Text colors */
    --ED-text-darkbg: var(--ED-gray-14); /* light text against dark bg */
    --ED-text-lightbg: var(--ED-gray-1); /* dark text against light bg */

    --ED-text-color: var(--ED-text-darkbg); /* Default is light text / dark bg */

    --ED-highlight-text: var(--ED-yellow-1);
    --ED-grayed-text: var(--ED-gray-8); /* de-emphasized text */

    /* Branding colors */
    --ED-brand: var(--ED-red-1);
    --ED-brand-2: var(--ED-yellow-1);

    /* Answer indicators */
    --ED-correct: var(--ED-green-1);
    --ED-incorrect: var(--ED-red-1);

    /* Surfaces / overlapping containers */
    --ED-surface-1: var(--ED-gray-1); /* farthest back */
    --ED-surface-2: var(--ED-gray-3);
    --ED-surface-3: var(--ED-gray-5);
    --ED-surface-4: var(--ED-gray-7);
    --ED-surface-5: var(--ED-gray-9); /* closest to user */

    /* Semantic indication of review vs. lesson */
    --ED-lesson-clr: var(--ED-red-1);
    --ED-review-clr: var(--ED-blue-1);

    /* Semantic indication of item types */
    --ED-radical-clr: var(--ED-red-1);
    --ED-kanji-clr: var(--ED-blue-1);
    --ED-vocab-clr: var(--ED-green-1);
    --ED-extra-clr: var(--ED-yellow-1);

    /* TODo: Box shadows */
}

/* ... */

/* RRW dark text better contrast here */
  .kotoba-table-list table {
    --ED-text-color: var(--ED-text-lightbg);
  }

  .kotoba-table-list table a span,
  .kotoba-table-list table a time,
  .kotoba-table-list table td span.pull-right {
    color: var(--ED-text-color);
  }

  .kotoba-table-list tr[class|="radical"],
  .kotoba-table-list tr[class|="kanji"],
  .kotoba-table-list tr[class|="vocabulary"] {
    background-image: none;
  }

  .kotoba-table-list tr[class|="radical"]:nth-child(odd),
  .kotoba-table-list tr[class|="kanji"]:nth-child(odd),
  .kotoba-table-list tr[class|="vocabulary"]:nth-child(odd) {
    filter: brightness(1.1);
  }

  .kotoba-table-list tr[class|="radical"] {
    background-color: var(--ED-radical-clr);
  }

  .kotoba-table-list tr[class|="kanji"] {
    background-color: var(--ED-kanji-clr);
  }

  .kotoba-table-list tr[class|="vocabulary"] {
    background-color: var(--ED-vocab-clr);
  }

/* ... */

This way you can theme any colors you want quite easily, just by editing the top rule.

Hopefully this makes sense. If all this is too much to take in as PRs, just let me know. I'm willing to create and maintain my own fork if you'd prefer.

I've been thinking that it might be wiser to take any theming of userscripts out of the base theme. Once the custom properties are stable for the top level theme, people can create their own user-styles for each plugin and just use the same custom props (which will inherit from from the parent). In other words: just one stylesheet for Extreme Dark itself, then another for ED-Ultimate-Timeline, another for ED-Self-Study, etc.

Since the user will install all of these locally, it won't be any slower to load. And managing the CSS rules for each userscript independently would be a whole lot more tractable than one monolithic script. ED itself is just responsible for setting the overall theme custom-props, and overriding the default WK selectors. Each script author (or whoever) can write their own CSS overrides for the theme.

I'm not decided on this yet, but I think it might make sense.

Sepitus-exe commented 1 year ago

I do love the new look of your kotoba-tables. Not only the text color but also the changes to the odd child filter. It is unreal how better it looks compared to the current state. Great job!

Reworking the color structure is a massive undertaking, that is definitely needed. I'm glad you took that on and would be happy to merge it into the main repository, but if you want to create and maintain your own fork for convenience's sake or any other reason, you are also welcome to do so. It was my intention from the very beginning to allow people to fork and tweak the theme to their liking.

I thought about theming userscripts outside the main file when I was working on the ultimate timeline, but back then decided to leave it incorporated to simplify installation and prevent confusion. After reviewing the code, it does make the main file significantly chunkier, 250 lines is way too much for a single script not everyone uses. I wouldn't be opposed to extracting script styles into their own dependant files. It would also make working in parallel on different scripts painless, compared to the current way of keeping changes confined to a single file.

Sepitus-exe commented 1 year ago

I won't be making any changes to the repository, for now, to make the potential merge as painless as possible and also due to some of my real-life responsibilities getting in the way of my free time. But don't take this as a push to prevent you from forking your own version, just more of a reassurance that things won't change while you work on your side.

wrex commented 1 year ago

Great!

Didn't want to get too pushy as I've been making some fairly significant changes (but hopefully ones that will make this more maintainable). Give me a few days to make any major structural changes to merge into the main branch. Then we can work on individual issues.

I think I see how you created this now: Just finding things page-by-page that you wanted to style, grabbing the selector from the browser inspector and going to town.

Unfortunately, the default WK CSS is a hot mess. A complete mishmash of tailwind, bootstrap, and their own rules. Pretty much everything has crazy-specific selectors that are super difficult to override.

Now that I'm working through rule by rule and seeing what each one does it's making more sense, but it pains me having to write such super ugly selectors. Two or more IDs in a selector is a clear indication that you're doing it wrong, but they are all over the place in the WK CSS.

Anyway, I'm making progress and it's looking pretty nice so far, but it's a slog for sure. I'm actually fixing some broken stuff in the WK code, not stuff in your code! (The kotoba-tables and the community forum stuff in particular)

wrex commented 1 year ago

My working branch is here if you want to see what I'm up to prior to filing the PR:

https://github.com/wrex/WKElementaryDark/tree/rrw-darken-meaning

wrex commented 1 year ago

The Issue #23 pr includes a folder for userscript-styles. It includes a stylesheet for self-study that I've been using successfully:

Screenshot 2023-01-17 at 5 14 34 PM

It's a separate stylesheet, but once added to stylus (with Extreme Dark loaded) it works well enough for me.

I think I should add defaults for the --ED- props in case Extreme Dark ISN'T loaded. That's probably a good practice moving forward.

Please leave this issue open until I commit a fix for it.

Suggestion for branch management

By the way, since you've given me commit privileges to this repository, I'd like to propose this process:

  1. Assign issues as we see fit.
  2. Never work in main, always create a branch off of main to work on each issue individually. Main should only get merge commits.
  3. Except for super trivial stuff, never merge your own code into main. Get another dev to review the code and perform the merge.
  4. Use issue comments to request a review and merge.
  5. Don't delete issue branches right away (if ever). History is valuable.
  6. Use issues for all communication about the code.

Thoughts?

Sepitus-exe commented 1 year ago

Since the current way is a free for all on the main branch. At least I approached it that way. This would for sure be an improvement to the work flow. Im for it.

I will add an review request flair for the issues so they are easy to identify.

wrex commented 1 year ago

Great minds think alike. I already created a "verify fix" tag for that purpose.

I propose we change the assignee to the person who should verify the fix when it's ready.

Sepitus-exe commented 1 year ago

Sounds good to me