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

add prop to style primary meaning and reading during reviews #119

Open
wrex opened this issue Mar 25, 2023 · 12 comments · May be fixed by #135
Open

add prop to style primary meaning and reading during reviews #119

wrex opened this issue Mar 25, 2023 · 12 comments · May be fixed by #135
Assignees

Comments

@wrex
Copy link
Collaborator

wrex commented Mar 25, 2023

As it stands the primary meaning is literally the lowest contrast text on your screen when you open the info panel during a review. The info you expressly ask for is the hardest to see (not optimal).

I'm going to try to tackle some of the changes today. I'll likely work on this as well.

@wrex wrex self-assigned this Mar 25, 2023
@wrex
Copy link
Collaborator Author

wrex commented Mar 25, 2023

I've implemented this in the branch https://github.com/Sepitus-exe/WKElementaryDark/tree/118-march23-update

Holy cats it makes a big quality of life improvement!

Before:

Screenshot 2023-03-25 at 12 49 55 PM

(It would have made more sense if I'd answered incorrectly, but try to find the primary meaning!).

After:

Screenshot 2023-03-25 at 12 51 42 PM

@Sepitus-exe
Copy link
Owner

While the solution to this issue has been included in the last PR and is now in the wild as a part of the live release, I will keep this issue open, since I wish to modify the implementation in the near future

@shdwns
Copy link

shdwns commented Apr 2, 2023

''Vocabulary Reading'' panel also should be the flat #282828 instead of the default WK gradient one

@wrex
Copy link
Collaborator Author

wrex commented Apr 3, 2023

''Vocabulary Reading'' panel also should be the flat #282828 instead of the default WK gradient one

I'm not sure what you're referring to. Review or lesson pages? The lesson pages have several PRs awaiting approval from @Sepitus-exe.

The review page styles it the div with the text "Vocabulary Reading" using --ED-reading (#282828) correctly, as shown in the comment above.

@shdwns
Copy link

shdwns commented Apr 3, 2023

I thought it supposed to look like the one on the bottom since the current one uses the default WK style in reviews/lessons
readingcard

@wrex
Copy link
Collaborator Author

wrex commented Apr 3, 2023

Ah, I see. Yes you're right. It's currently being overriden with this more specific rule:

.quiz-input__question-type-container[data-question-type=reading] {
    background-image: linear-gradient(#3C3C3C, #1A1A1A);
    border-top: 1px solid #555;
    border-bottom: 1px solid #000;
    color: #fff;
    text-shadow: 0 1px 0 rgba(0,0,0,0.2);
}

@Sepitus-exe This is pretty easy to fix, but I'll let you handle it since I'm unsure what you were referring to here

@wrex
Copy link
Collaborator Author

wrex commented Apr 5, 2023

'Vocabulary Reading'' panel also should be the flat #282828 instead of the default WK gradient one

The fix for this is included in PR #136

You can manually install this version until @Sepitus-exe has time to review all the changes:

https://raw.githubusercontent.com/Sepitus-exe/WKElementaryDark/march-unified/WKElementaryDark.css

@shdwns
Copy link

shdwns commented Apr 5, 2023

Uhh, i think the new update brought some new problems...
This one happens in review
image
And this one also looks somewhat weird
image

@wrex
Copy link
Collaborator Author

wrex commented Apr 5, 2023

It would be helpful if you could describe what you expect to see and what you mean by "new update".

I have no idea what version of the WKED stylesheet you are running.

The default WK stylsheet (what you'll see if you disable all stylus stylsheets) has a pretty dramatic hover effect. I think that might be what you are seeing in the first image. If you move your mouse it should go back to normal size.

What exactly is weird in the second image? It appears to be rendering as intended (with --ED-radical background color for the radicals)

@shdwns
Copy link

shdwns commented Apr 5, 2023

https://raw.githubusercontent.com/Sepitus-exe/WKElementaryDark/march-unified/WKElementaryDark.css

I was talking about this one, I installed it through copying and pasting the whole code in a new stylesheet. I don't know if that is the reason for the bug
Screenshot_11
So this is the default wanikani
Screenshot_12
And this is the official WKED that is on userstyles.world
Screenshot_14
and this is the one i installed myself

@shdwns
Copy link

shdwns commented Apr 5, 2023

The default WK stylsheet (what you'll see if you disable all stylus stylsheets) has a pretty dramatic hover effect. I think that might be what you are seeing in the first image. If you move your mouse it should go back to normal size.

image
this is without hovering
image
and this is with hovering
these are PR #136
image
without hover
image
with hover
and this is the one that is on userstyles.world

@wrex
Copy link
Collaborator Author

wrex commented Apr 5, 2023

Thanks. That's helpful.

I think @Sepitus-exe has other priorities at the moment. It's not my repository, so bear with us for a few more days.

I'll use the following names:

  • "Vanilla" = the default WK styling (after the recent update)

  • "Published" = WKElementaryDark.css currently on userstyles.world. This is pretty broken since the update. I think it includes my first emergency patch, but none of the six outstanding PRs

  • "Unified" = WKElementaryDark.css from my "march unified" branch that merges the other five PRs into one.

For some bizarre reason, the vanilla WK style applied a fixed 44px width and height to the radicals in the Radical Combination section. For reasons I'm not completely sure about, this caused the published WKED stylesheet to show the radicals in the "Radical Combinations" section to appear small and shifted off of the baseline. So I removed the fixed width and height in the unified patch.

Unfortunately, the vanilla styles also override the background color (so it's a surface color, not the background color).

The unified patch is at least legible and understandable, so I'm loathe to create another PR until @Sepitus-exe has some time to review.

I created issue #137 to track this. I've actually already written the code to make it look like the following, but I won't submit a PR for it yet:

normal:
Screenshot 2023-04-05 at 10 41 57 AM

hover:
Screenshot 2023-04-05 at 10 42 06 AM

(Requires adding an !important tag to the background color, and adding some padding to this rule:

.page-header__icon--radical, .radical-highlight, .component-character--radical .component-character__characters {
    background-color: var(--ED-radical) !important;
    background-image: none;
    padding: 0.2em;
}

Going forward, please create separate issues for anything else you find. This issue is about something else entirely.

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 a pull request may close this issue.

3 participants