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

GitHub alert block quotes #89

Closed
2 tasks done
jannis-baum opened this issue Jul 13, 2024 · 16 comments · Fixed by #118
Closed
2 tasks done

GitHub alert block quotes #89

jannis-baum opened this issue Jul 13, 2024 · 16 comments · Fixed by #118
Assignees
Labels
topic:syntax e.g. functional Markdown extensions type:feature New feature or request

Comments

@jannis-baum
Copy link
Owner

jannis-baum commented Jul 13, 2024

  • add markdown-it plugin for rendering
  • figure out colors

OP @Tweekism in #87 (comment)

Right, so the reason I came looking for iamcco in the first place was to get these things working...

Screenshot from 2024-07-14 01-51-37

I have a young lady working in my office and she likes things to be pretty.

This is how it looks on GitHub:

Screenshot from 2024-07-14 01-52-44

Soooo.... add it to the list? 😅

Research from @tuurep

More information about those:

https://github.com/orgs/community/discussions/16925

Possible markdown-it plugin:

https://github.com/antfu/markdown-it-github-alerts

I think it goes more into the "additional" markdown but I would definitely be down to adding this

@jannis-baum jannis-baum added type:feature New feature or request topic:syntax e.g. functional Markdown extensions labels Jul 13, 2024
@Tweekism
Copy link
Collaborator

Can I start looking into this? or are you already on it @tuurep

@jannis-baum
Copy link
Owner Author

jannis-baum commented Jul 14, 2024

Just as a quick disclaimer before anyone starts working on this:

In #72 we update all dependencies and in #78 I change Vivify from CommonJS to ESM which changes a few things around how importing modules works. So if you want to start working on this now, either be prepared to rebase things and solve the merge conflicts or branch off from after those changes were made, but then be aware that force pushes may still happen under your branch-off point

I'll split up #78 into two PRs so we can merge the things that may be annoying for other features sooner


Edit I made the split. The order of this will now be

  1. Merge Update dependencies & README #72 after resolving the discussion with @tuurep
  2. Someone can review ESM and simplified config #90 and then we merge it

#78 is now separate and built on top of #90 and no longer needs to be regarded in this. If anyone wants to start working on this issue (or any other one that installs new modules) I would suggest branching off from where #90 is. But again, changes may still happen there

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

This will probably be really simple, only thing I'm concerned about is dark theme colors :D

For light theme we can copy what Github uses:

alert type left-side-border and title font color
Note #0969da
Tip #1a7f37
Important #8250df
Warning #bf8700
Caution #cf222e

The way Github names these color variables also reveals to me that they attach some meaning to these colors and reuse them in their UI:

  • #0969da = --fgColor-accent
  • #1a7f37 = --fgColor-success
  • #8250df = --fgColor-done
  • #bf8700 = --fgColor-attention
  • #cf222e = --fgColor-danger

@jannis-baum does the dark theme have some existing colors that could fit? If not, I can cook up some suggestions.

Edit:

We could see how the ANSI colors look on it:

https://github.com/jannis-baum/dotfiles/blob/60e769172e2c2740999c3e7892a7530c75bd6a26/.lib/nosync/color-schemes/jellyfish.yaml#L57

blue-green-magenta-yellow-red

@jannis-baum
Copy link
Owner Author

This will probably be really simple, only thing I'm concerned about is dark theme colors :D

For light theme we can copy what Github uses:

alert type | left-side-border and title font color

:---------:|:------------------------------------:

Note | #0969da

Tip | #1a7f37

Important | #8250df

Warning | #bf8700

Caution | #cf222e

The way Github names these color variables also reveals to me that they attach some meaning to these colors and reuse them in their UI:

  • #0969da = --fgColor-accent

  • #1a7f37 = --fgColor-success

  • #8250df = --fgColor-done

  • #bf8700 = --fgColor-attention

  • #cf222e = --fgColor-danger

@jannis-baum does the dark theme have some existing colors that could fit? If not, I can cook up some suggestions.

Edit:

We could see how the ANSI colors look on it:

https://github.com/jannis-baum/dotfiles/blob/60e769172e2c2740999c3e7892a7530c75bd6a26/.lib/nosync/color-schemes/jellyfish.yaml#L57

blue-green-magenta-yellow-red

Yes, that sounds good! And actually, now that you mention ANSI, we should also use those colors for Notebooks. There we also have to render ANSI and the current default colors are very ugly

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

Ok, but there's a chance a separation of the Alert colors and ANSI colors would be best, for example if we look at someone's take on an Alacritty Github Light theme:

image

They're way quite different from the ones used in the alerts

@jannis-baum
Copy link
Owner Author

Are they? Haha to me they honestly look pretty similar👀 Maybe the yellow is a bit off but still doesn't seem too much to me. Is there a specific one that you think is off by a lot or am I just not sensitive enough to the colors?

@tuurep

This comment was marked as outdated.

@tuurep

This comment was marked as outdated.

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

Wait what am I looking at 😄

https://github.com/alacritty/alacritty-theme/blob/master/themes/github_light.toml

This is very different, lets see

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

image

Yeah I mean, this is what I thought I saw in the original screenshot, with the magenta being very dark

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

Sorry I got confused by some outdated file there.

I still would favor distinguishing the (error) syntax highlighting and the alert colors

I'm obsessed with stuff like this so I can help you out, is that ok? 😄

@jannis-baum
Copy link
Owner Author

I still would favor distinguishing the (error) syntax highlighting and the alert colors

I'm obsessed with stuff like this so I can help you out, is that ok? 😄

Haha yes, of course! If it's important to you let's separate them for sure :)

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

I investigated more and turns out this:

image

(source: https://github.com/projekt0n/github-theme-contrib)

is more up to date than this:

image

(source: https://github.com/alacritty/alacritty-theme)

So actually I'm leaning towards this suggestion again:

So maybe lets try using ANSI for the Alerts as is on the dark theme, then see if something emerges like

  • "this is unreadable, increase contrast"

adjust as needed?

So plan of action:

  1. Add the plugin
  2. Add "known" colors for Alerts (light theme)
  3. Add ANSI colors for Alerts (dark theme)
    • if some look bad, adjust a bit
  4. ???
  5. Think about IPython error syntax highlighting (in a separate issue preferably)

@jannis-baum
Copy link
Owner Author

Sounds like a good plan to me! :) I'll open the IPython-related issue, would you work on that one as well?

I can do #87 next then if you're working on these :)

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

I'll open the IPython-related issue, would you work on that one as well?

Yeah but could you describe your biggest gripes in:

There we also have to render ANSI and the current default colors are very ugly

I've only taken a brief glance and wasn't overly horrible on Light Theme, but does it use the same colors for both Light and Dark?

@jannis-baum
Copy link
Owner Author

I'll open the IPython-related issue, would you work on that one as well?

Yeah but could you describe your biggest gripes in:

There we also have to render ANSI and the current default colors are very ugly

I've only taken a brief glance and wasn't overly horrible on Light Theme, but does it use the same colors for both Light and Dark?

I replied to this on the other issue #101

tuurep added a commit to tuurep/Vivify that referenced this issue Jul 22, 2024
tuurep added a commit to tuurep/Vivify that referenced this issue Jul 22, 2024
tuurep added a commit to tuurep/Vivify that referenced this issue Jul 22, 2024
tuurep added a commit to tuurep/Vivify that referenced this issue Jul 22, 2024
tuurep added a commit to tuurep/Vivify that referenced this issue Jul 22, 2024
tuurep added a commit to tuurep/Vivify that referenced this issue Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:syntax e.g. functional Markdown extensions type:feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants