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

inherit the colors for correct and mistake faces from success and warning respectively #40

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

yilkalargaw
Copy link
Contributor

Since there is a huge variance in the visibility of green and red in various themes it might be better to inherit the colors for speed-type-correct and speed-type-mistake from the the success and warning faces which are usually set by the theme being used. The reason to not inherit from them directly from those faces was in-case the theme does something weird like using a different weight or having boxes etc...

@dakra
Copy link
Owner

dakra commented Sep 26, 2023

Thanks.
On my theme (moe-theme) I like (face-foreground 'error) better than 'warning for speed-type-mistake because that's red while warning is orangy.

But I didn't test with any other theme so I don't have too strong of an opinion but personally I would prefer 'error just because it looks better for me.
WDYT?

@yilkalargaw
Copy link
Contributor Author

To be frank i forgot about error. That would be better. I'll make the changes.

Since there is a huge variance in the visibility of green and red in
various themes it might be better to inherit the colors for
speed-type-correct and speed-type-mistake from the the success and
warning faces. The reason to not inherit from them directly was
in-case the theme does something weird like using a different weight
or having boxes etc...
@dakra dakra merged commit 28b8e8c into dakra:master Sep 26, 2023
2 of 3 checks passed
@dakra
Copy link
Owner

dakra commented Sep 26, 2023

🙏 Thanks

@yilkalargaw yilkalargaw deleted the patch-1 branch September 29, 2023 11:40
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.

2 participants