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

Revise Notebook colors #101

Open
2 tasks
jannis-baum opened this issue Jul 16, 2024 · 22 comments · May be fixed by #152
Open
2 tasks

Revise Notebook colors #101

jannis-baum opened this issue Jul 16, 2024 · 22 comments · May be fixed by #152
Assignees
Labels
topic:design focussed on CSS-related things type:enhancement Adding to/changing existing feature

Comments

@jannis-baum
Copy link
Owner

jannis-baum commented Jul 16, 2024

continued discussion from #89

  • ANSI colors that are parsed by ansi_up
  • error outputs, .ipynb-output-stream-stderr and .ipynb-output-error in CSS
@jannis-baum jannis-baum added type:enhancement Adding to/changing existing feature topic:design focussed on CSS-related things labels Jul 16, 2024
@jannis-baum
Copy link
Owner Author

@tuurep feel free to add to the list above if there's anything else and assign yourself to the issue if you want to work on this :)

@jannis-baum jannis-baum added this to the v.0.2.0 milestone Jul 16, 2024
@jannis-baum
Copy link
Owner Author

@tuurep

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 created a branch for this issue and added some more code to the notebook to look at the main colors. As you suspected, especially in dark mode it looks bad right now:

image

@tuurep
Copy link
Collaborator

tuurep commented Jul 16, 2024

Thanks, it's good info! I'd be interested looking into this but only after a few other things first.

As for the borderline-unreadable white-on-yellow error text, I remember seeing this issue in the past:

ipython/ipython#13446

If that ends up a problem, will be useful to look there first.

@jannis-baum
Copy link
Owner Author

Interesting, good research on that!

I'd be interested looking into this but only after a few other things first.

Nice! What's your agenda for the next things you want to do on Vivify? Just to keep an overview :)

@tuurep
Copy link
Collaborator

tuurep commented Jul 17, 2024

I'll first review your config PR, then add GH alerts plugin

After that I think I'll take a look at this issue to know whether it's simple or complicated

And my two issues on the vim plugin repo are constantly on my mind :D

I've been using a branch for myself which refreshes on TextChanged and TextChangedI and it has worked well enough but would be great to do it properly

@jannis-baum
Copy link
Owner Author

jannis-baum commented Jul 17, 2024

I'll first review your config PR, then add GH alerts plugin

After that I think I'll take a look at this issue to know whether it's simple or complicated

Thank you! Sounds good!

And my two issues on the vim plugin repo are constantly on my mind :D

Oh, right, I forgot about those! While vivify.vim#5 is definitely in the right place, the good solution for vivify.vim#6 definitely needs more work here (adding the query parameter for initial scroll). Should we open an issue for that here then? I can also implement that if you want it :) There's also another API endpoint I will need here to do my project of a customized Jupyter server with a Vim plugin to edit Jupyter Notebooks in Vim and display them in Vivify so I could implement both of those API features in one go.

I've been using a branch for myself which refreshes on TextChanged and TextChangedI and it has worked well enough but would be great to do it properly

Ah, yes, I just read up again on that issue and saw that we got a bit stuck on the other issue of files that are too large. This can also be split into two if you want: one to give the option to refresh on different events which will take care of your use case, and second new one for the max file length and replacing the commandline argument to curl with the temporary file; you can just do only the first, no need to worry about the second now :)


EDIT I opened #103 for vivify.vim#6 and vivify.vim#7 for the curl thing. I will probably look into #103 first thing after we have released v0.2.0

@tuurep
Copy link
Collaborator

tuurep commented Jul 17, 2024

the good solution for jannis-baum/vivify.vim#6 definitely needs more work here (adding the query parameter for initial scroll). ... I can also implement that if you want it :)

Yeah thank you! I didn't know where to begin with this so would be fantastic.

one [issue] to give the option to refresh on different events

Nice, would be good to just add this. I'll ask a question on issue jannis-baum/vivify.vim#5

@jannis-baum jannis-baum removed this from the v.0.2.0 milestone Jul 18, 2024
@Tweekism
Copy link
Collaborator

To add my 2 cents in to the "next things" discussion, the update on key press / real time preview thing is top of my list too. That's probably the last major feature missing from the old plugin?

On a totally meta note, I wonder if we need to do something in the Google analytics / SEO area as this doesn't show up at all for anyone searching along the lines of neovim, markdown, preview, etc.

I've also been keeping an eye on the old plug-in, talking to people there and pointing them this way nudge nudge etc. Last guy i spoke too said it looks nice but he is on Windows. I know there is already discussion elsewhere on the viability of Windows so no need to rehash that here.

@tuurep
Copy link
Collaborator

tuurep commented Jul 24, 2024

Well you're in for some good news because

the update on key press / real time preview thing is top of my list too.

This is what I did just last night :D

But it needs review: jannis-baum/vivify.vim#10

If you wanna test it works as you expect as well, would be great!

@tuurep
Copy link
Collaborator

tuurep commented Jul 24, 2024

I think one issue with searchability is that the name Vivify is not very unique, when you try searching in google:

image

@Tweekism
Copy link
Collaborator

Well you're in for some good news because

This is what I did just last night :D

Oh sweet! I'm off to bed now (midnight here) but will def have a look in the morning. Is it just a branch on your repo I can pull from?

I think one issue with searchability is that the name Vivify is not very unique, when you try searching in google:

Well I didn't even think to search vivify, or even stand alone Markdown viewers for that matter. For me, I was looking specifically for a neovim plugin to replace the "built-in" one coming from vscode, not even realising the benefits of it being a standalone thing until it was presented to me here. And i think a lot of people would have that same perspective.

vivify.nvim probably needs to have Markdown in the title somehow.

@tuurep
Copy link
Collaborator

tuurep commented Jul 24, 2024

Is it just a branch on your repo I can pull from?

Yep, over here: https://github.com/tuurep/vivify.vim/tree/configure-refresh-speed

@Tweekism
Copy link
Collaborator

Cool, do I need to do and configurating with parameters. Or can I just clone it down and point Lazy at the local repo?

@tuurep
Copy link
Collaborator

tuurep commented Jul 24, 2024

It makes it so that the new refresh event (refresh on TextChanged) is the default so nothing needs to be set

And to get the old behavior (refresh on CursorHold), set:

let g:vivify_instant_refresh = 0

So if you can get nvim to use this branch, should be good

@Tweekism
Copy link
Collaborator

Ok, so it is nearly 1 am here now, but you got me excited enough to go downstairs and get my laptop.

So far this is looking really sick, works, no problems. I'll hit it up more in my normal workflow tomorrow and let you know how it goes, but so far, loving it mate!

@tuurep
Copy link
Collaborator

tuurep commented Jul 24, 2024

Great, thanks a lot :)

@jannis-baum
Copy link
Owner Author

Hey guys! I will try and look at @tuurep's PR on the Vim repo ASAP but might be that I still need until tomorrow.

For the SEO/visibility I totally agree that we should look into that. I've been wanting to open a discussion topic for it but haven't gotten around to doing it. Thanks @Tweekism for nudging people haha, I assume that's where people have been coming from. Also good idea on including more Markdown-mentions on the Vim repo, I will do that soon!

@Tweekism
Copy link
Collaborator

Hey man, no rush!

I'm happy to test it like this for as long as needed. Anyone new coming in to the project I'm sure will be ok to wait a couple days :)

@jannis-baum
Copy link
Owner Author

For the dark theme here we can probably actually take all colors from Jellyfish. Before we do this one it probably makes sense to look at #120 though

@jannis-baum
Copy link
Owner Author

Hey @tuurep since you are looking into the icons next, should I take over this one and (before) the CSS refactor #120?

@tuurep
Copy link
Collaborator

tuurep commented Jul 30, 2024

If you want, sure!

Also forewarning: I have no clue or strong feelings about the CSS refactor so any suggestions about that are very welcome if you've got opinions 😄

I'm kinda in a zone of "it works and I wouldn't really care to touch it"

@tuurep
Copy link
Collaborator

tuurep commented Jul 30, 2024

Oh I see, you've commented about just that 30 min ago. I'll check that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:design focussed on CSS-related things type:enhancement Adding to/changing existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants