Skip to content
This repository has been archived by the owner on Mar 1, 2023. It is now read-only.

syntax highlighting in inline context blocks #19

Open
hmaurer opened this issue Apr 7, 2021 · 11 comments
Open

syntax highlighting in inline context blocks #19

hmaurer opened this issue Apr 7, 2021 · 11 comments

Comments

@hmaurer
Copy link
Contributor

hmaurer commented Apr 7, 2021

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:

Screenshot 2021-04-07 at 10 42 49

Thoughts?

@mattpocock
Copy link
Owner

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?

@hmaurer
Copy link
Contributor Author

hmaurer commented Apr 7, 2021

Here's another attempt (though I'm still not happy with it; the contrast is a bit weak):

Screenshot 2021-04-07 at 11 46 05

I am wondering if perhaps it'd be better to just show the name of the context key inline (e.g. itemBeingHeld) and show the content on hover in a popover (formatted). The downside is that it wouldn't be possible to see the value at a glance.

@bemayr
Copy link
Contributor

bemayr commented Apr 7, 2021

Nice improvement! 👏
Regarding the hover idea, I was already thinking of this relating to the different event payloads (#15 (comment)), but one reason why :hover` is bad, is its problem with touch interaction.
During research or while explaining stuff to someone I often use my convertible's touch screen and then all the hover-features suddenly get inaccessible.

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?

@hmaurer
Copy link
Contributor Author

hmaurer commented Apr 7, 2021

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!.

@mattpocock
Copy link
Owner

mattpocock commented Apr 7, 2021

@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
}

@mattpocock
Copy link
Owner

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.

@mattpocock
Copy link
Owner

@all-contributors please add @hmaurer for ideas, code

@allcontributors
Copy link
Contributor

@mattpocock

@hmaurer already contributed before to ideas, code

@hmaurer
Copy link
Contributor Author

hmaurer commented Apr 7, 2021

@mattpocock yeah that's what I did with the theme. In the example above I used atom-one-light and wrapped every class in .hljs-inline. I'll open a PR and we can iterate from there with the Vercel preview!

@mattpocock
Copy link
Owner

@hmaurer Gotcha, nice.

@bemayr
Copy link
Contributor

bemayr commented Apr 7, 2021

I just checked the current version and really like this change, because after this got merged context won't look like the sendable events anymore!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants