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

Use a lighter style for <kbd> #536

Merged
merged 1 commit into from
Jan 8, 2019
Merged

Use a lighter style for <kbd> #536

merged 1 commit into from
Jan 8, 2019

Conversation

Xenonym
Copy link
Contributor

@Xenonym Xenonym commented Jan 5, 2019

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X] Enhancement to an existing feature

Fixes #532.

What is the rationale for this request?
The current style for <kbd> is white on black text, which gives it too much contrast relative to its surrounding content:

White on black <kbd> style

What changes did you make? (Give an overview)
The style of <kbd> was changed to use a lighter, black on white style instead:

Black on white <kbd> style

Provide some example code that this change will affect:

<kbd>Ctrl</kbd> + <kbd>C</kbd>

Testing instructions:

  1. Use the <kbd> tag on a MarkBind page. It should render with the new, lighter style.

Copy link
Member

@yamgent yamgent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I am not so sure whether it is good to include it in asset/css/markbind.css (which will affect all MarkBind websites), rather than in docs/css/main.css (which will only affect our documentation website).

I am fine with the current way as it is, unless there's some objection similar to above.

@Chng-Zhi-Xuan
Copy link
Contributor

Although not in the scope of this PR, should we support both light and dark themed <kbd>?

I feel its ok to override the <kbd> style in markbind.css since MarkBind websites are initialised with a white background, it blends in with the updated Bootstrap 4 colour styles as well.

@yamgent yamgent added this to the v1.15.3 milestone Jan 6, 2019
@yamgent
Copy link
Member

yamgent commented Jan 6, 2019

I feel its ok to override the <kbd> style in markbind.css since MarkBind websites are initialised with a white background, it blends in with the updated Bootstrap 4 colour styles as well.

👍

Although not in the scope of this PR, should we support both light and dark themed <kbd>?

That can be considered, although I am not an expert in css so I was wondering what you had in mind for that kind of implementation. I was thinking that if the author is going to use their own styles.css, they will be overriding the <kbd> with their own anyway.

@Xenonym
Copy link
Contributor Author

Xenonym commented Jan 6, 2019

@yamgent it would be pretty straightforward to have a white or black class for <kbd> and have the user pick using <kbd class="white">. However, I feel that the default CSS we include in MarkBind should be just providing sensible defaults, and not be including multiple variations for elements. As you say, the user can easily override these defaults on their own.

@acjh
Copy link
Contributor

acjh commented Jan 6, 2019

Theming files might be better. See discussion in #386.

For example, we could put this override in a theme-light.css and include it by default.
However, author should be able to remove it so that one theme file will not affect another.
As mentioned in #386, there should not be much code change as it's just including CSS.

@yamgent yamgent modified the milestones: v1.15.3, v1.16.1 Jan 6, 2019
@yamgent yamgent merged commit e29a1a0 into MarkBind:master Jan 8, 2019
@yamgent yamgent changed the title Use a lighter style for <kbd> Use a lighter style for <kbd> Jan 8, 2019
@Xenonym Xenonym deleted the lighter-kbd-style branch January 8, 2019 12:14
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.

Use a lighter style for <kbd>
4 participants