-
Notifications
You must be signed in to change notification settings - Fork 10
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
Style code blocks #210
base: main
Are you sure you want to change the base?
Style code blocks #210
Conversation
To clarify the problem look at this example were I have hard-coded the background color: And here are the code blocks using the default (I think it comes from highlight.js) background color: Looking at these, I think it's a no-brainer to go with the default code background. The difference is not big for the light theme, and it looks reasonable for the dark theme. The border color is sub-optimal on the dark-theme though :/ As I said above, we could set different colors (keep this one for light theme and select a darker one for dark theme) for our two themes, but this wouldn't play well with for example NimiTheme. |
this looks cool!
agree
also agree
yeah, it would be hard to check every stuff in nimitheme how it is impacted. should we keep an option to revert to old style, maybe (although I am starting to dislike complexity of all these options, at some point we will need to pay some debt and simplify stuff)? I feel a better solution might like in css variables or something like that but I do not know enough about it (and we do not need to work on this now). As further feedback if possible I would make sure that a nb-code not followed by a nb-output should have rounded corners (I guess it should not be too hard with an override using some css selector...). and thanks for working on this, it definitely looks like an improvement! |
src/nimib/renders.nim
Outdated
@@ -20,6 +20,8 @@ proc useHtmlBackend*(doc: var NbDoc) = | |||
doc.partials["nbCapture"] = """{{>nbCodeOutput}}""" | |||
doc.partials["nbCodeSource"] = "<pre><code class=\"nohighlight hljs nim\">{{&codeHighlighted}}</code></pre>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this line and the following be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(from here it is not clear but later lines seem to override this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct 👍 Will delete them
Yes, it's a hard nut to crack :/ A plugin system could solve part of the problem as you could choose some styles but not others to use in your theme. For example, if I want to default nimib style of images but wanted my custom style of nbCode. CSS-variables could actually work though 🤔 They would need to be defined before the styles though, so it would require us adding a dedicated
Yes it should be solvable in mustache using different classes, but I'm not sure about how it could be done using only CSS. |
The code ain't pretty, but the code blocks are 🙃 I'll fix the tests now. |
Previously, we haven't really styled the code and output as a single unit, but now that we do, I think it would make sense to put them inside a What I suggest is basically that we define: doc.partials["nbCode"] = """
<div class="nb-code-wrapper">
{{>nbCodeSource}}
{{>nbCodeOutput}}
</div>""" This could potentially break custom styles but I don't think the risk is that high. What do you say? |
I fixed another thing that has been a bit ugly: overflowing outputs! Penguins looks really slick now: https://c8ca2d62fa794955ee68--nimib.netlify.app/penguins |
In the end you did not need this |
That looks much better! Thanks for looking in to this 😄 I'll try and implement this today!
Yes, if it looks decently good at least, I'm all for using variables. We just have to document somewhere that themes must define
Yeah, I've noticed :/ I could give it a shot trying to do it without classes, but I don't think you can select a parent using CSS currently (i.e. I can't style the
So yes, I think it's hard to solve at the moment :/
No, we can still get things like this: |
We have a problem, I picked a few random nimitheme themes and none of them defined neither |
Well we can use the current values as fallback values: https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties#custom_property_fallback_values We should indeed document which variables we use for our custom nb styling |
Ohh, good find! But we still have problems. We can only set one fallback value. So we are back to the problem of light and dark themes. The dark themes would look horrible if we chose a light background as fallback 🤔 |
And I have checked, there seems to be no way of doing "if this variable doesn't exist, use the previously defined value". It can only be a specific value, or the default value. |
Well our default is light and dark is set by useDark, maybe something can be done taking advantage of that? Anyway I think this is not too bad if something needs to be fixed either in nimitheme or customly, we can only do so much. |
Not sure tbh, it's only the default water.css themes that use
As long as that "something" isn't too cumbersome for @neroist to change, I agree. One big problem is also that nimitheme has the feature of choosing the highlight.js theme, which we will screw over here with our override of the code background which they can't do anything about. Maybe the solution is to not change the background manually, but instead choose a better default highlight.js theme for water.css dark mode? Then we only have to care about the border-color, which is more forgiving. |
This is my attempt at implementing code blocks that look something like the DeepNote blocks in #65 . The styling still needs some tuning (feedback is welcome) and there are some things we might have to consider. For example the colors (the background color of the code and the border color), if I hardcode them as I do now, they will look bad in dark mode for example. Add to that all themes in NimiTheme. If we hard-code them, it may look weird with any theme but the default light water.css theme. So we might have to go with using the default background color of the current theme. The border color could probably work if we set it to a grey-ish color, otherwise we get a fully black/white border which is not a good look.
@pietroppeter What is your opinion on this?