-
-
Notifications
You must be signed in to change notification settings - Fork 63
syntax highlighting in inline context blocks #19
Comments
Yes, for sure. From a design point of view it needs a more subtle treatment. I.e. light gray etc. Maybe HighlightJS has some different themes we could try? Or create our own? |
Here's another attempt (though I'm still not happy with it; the contrast is a bit weak): I am wondering if perhaps it'd be better to just show the name of the context key inline (e.g. |
Nice improvement! 👏 I think that aligned with @mattpocock's idea of this project being a transparent learning resource, the inline context should not be larger than what can actually be nicely in-lined and everything larger should be displayed in a block. What are your opinions here? |
Ah yes, you are totally right re touch interactions. This could be solved by showing the popover both on click/tap and on hover, but you'd still have the issue that it'd be hidden from a glance. Makes sense re keeping the inline context small!. |
@hmaurer Keep going, this is good. Perhaps turning this file into a CSS module, and wrapping all the classes therein in a CSS Module would work. https://github.com/mattpocock/xstate-catalogue/blob/master/styles/atom-one-dark.css I.e: .jsonModule {
.hljs {
display: block;
overflow-x: auto;
padding: 0.5em;
color: theme('colors.gray.200');
}
// Other hljs bits
} |
Also, I agree - inline context should be kept small. Part of the reason it's so sweet is that you can see it change on the page, without needing a hover. |
@all-contributors please add @hmaurer for ideas, code |
@hmaurer already contributed before to ideas, code |
@mattpocock yeah that's what I did with the theme. In the example above I used |
@hmaurer Gotcha, nice. |
I just checked the current version and really like this change, because after this got merged |
I implemented syntax highlighting for the "whole context" block (c.f. #18) and I am wondering if it'd make sense to also implement it for inline context. It could look something like this:
Thoughts?
The text was updated successfully, but these errors were encountered: