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

Consider adding HTML kbd styles #269

Closed
Eiji7 opened this issue May 13, 2021 · 13 comments
Closed

Consider adding HTML kbd styles #269

Eiji7 opened this issue May 13, 2021 · 13 comments

Comments

@Eiji7
Copy link

Eiji7 commented May 13, 2021

I have few use cases for mentioning keyboard shortcuts, for example:

<kbd>Ctrl</kbd>+<kbd>A</kbd>

Unfortunately currently in LiveBook there is no difference between them and rest of text. Of course markdown would be helpful here, but a different look may took attention of the reader which is important in such use case.

I would like to have some simple, but nice looking styles like on this site:
https://auth0.github.io/kbd/

Github also have a custom look for it, see: Ctrl+A. For more information take a look at HTML TAGS.

Since it's only about few simple CSS rules I believe it could be added in no time. Looking forward for your opinion.

@josevalim
Copy link
Contributor

How do you add HTML to Livebook today? I thought we discarded all HTML in markdown cells?

@Eiji7
Copy link
Author

Eiji7 commented May 13, 2021

Oh, is that so? Please take a look at this screen
screen

@josevalim
Copy link
Contributor

If you save the notebook to disk, does it preserve the HTML tags? What if you close the session and reopen it from file?

Or maybe I am misremembering.
We keep the tags but not the attributes? In any case, whatever we do, we need to mirror on the front-end Markdown parser.

@Eiji7
Copy link
Author

Eiji7 commented May 13, 2021

ok, things are going to be interesting …

This is content of saved file (same input):

# Test HTML

## Test section

<kbd >
Ctrl
</kbd>

and here is how it looks after opening file:
screen

@josevalim
Copy link
Contributor

Awesome, thanks. So I am probably misremembering. Let's see what @jonatanklosko has to say on our official stance for HTML tags in Markdown.

@jonatanklosko
Copy link
Member

We actually keep the HTML and on the client we sanitize it when rendering, so that any script-like stuff is deactivated. The problem with <kbd>Ctrl</kbd>+<kbd>A</kbd> is that Earmark only parses the first tag.

iex> EarmarkParser.as_ast("<kbd>Ctrl</kbd>+<kbd>A</kbd>")
{:ok, [{"kbd", [], ["Ctrl"], %{verbatim: true}}], []}

@Eiji7
Copy link
Author

Eiji7 commented May 13, 2021

Looks like it's a bug in Earmark: pragdave/earmark#356.

@josevalim
Copy link
Contributor

Cool, so we should probably add the styles for <kbd> and open up a separate issue to track the earmark limitation.

@jonatanklosko
Copy link
Member

jonatanklosko commented May 13, 2021

Sounds good! Actually I've just realised that this problem is only noticeable if the text starts with an opening HTML tag. When the tags are in the middle of the text, then it's all parsed as a regular paragraph:

iex> EarmarkParser.as_ast("You can use a kbydingin like <kbd>Ctrl</kbd>+<kbd>A</kbd>")
{:ok,
 [{"p", [], ["You can use a kbydingin like <kbd>Ctrl</kbd>+<kbd>A</kbd>"], %{}}],
 []}

When using such inline decorators, it's usually in the middle of the text, so realistically it's not much of a problem. I've just opened an issue regarding the edge case (RobertDober/earmark_parser#39).

Please let me know if you have any comments on the styling :)

image

@Eiji7
Copy link
Author

Eiji7 commented May 13, 2021

What do you think about setting also box-shadow to 1px 1px 1px color where color is for example black, gray or #777?

@jonatanklosko
Copy link
Member

There already is a tiny inner shadow, but we could make it more prominent, like this:

image

WDYT?

@Eiji7
Copy link
Author

Eiji7 commented May 13, 2021

Looks good for me.

@jonatanklosko
Copy link
Member

Cool, added in eea546d :)

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

No branches or pull requests

3 participants