Update water.css and fix code output

[dlesnoff]

Here is a PR with the changes proposed by @HugoGranstrom in #184 . Fix #184 .

[HugoGranstrom]

Nice! The only thing would be that we should create a specific class if we want to use samp for something else in the future. Or what do you think @pietroppeter?

[pietroppeter]

I do not see what has changed I suspect it might be different on different OSes, can we add screenshots of before and after? In particular on my side I still see the back tick content with a bigger font than the rest of the text as before (which is not what I see in water.css page). What was the change that samp as display was supposed to fix (for the moment I would not bother to make output a class, if we need we can do it later)? Should we also try removing normalize.css?

[HugoGranstrom]

Oh, the bigger font size is because of normalize.css still being used. The samp previously just looked like a inlined backticked string spanning multiple lines (ugly). Now it takes up the full width of the page.

[pietroppeter]

Ah I see about the samp. Would it make sense instead to wrap it in a pre block?

[pietroppeter]

Mmh actually I see it is already wrapped in a pre block with class nb-output. I guess that did not solve it? I actually never noticed this issue about the samp.

[pietroppeter]

mmh, actually reading about samp I see it is supposed to be an inline element. I guess we could skip the samp and wrap only in pre?

[dlesnoff]

On Linux, if I change only the water.css, I get an horrible code output. I am no CSS expert, but I guess we can wrap output both by samp and pre ? Is the display: block trick not functioning on your end?

Le lun. 6 mars 2023 à 09:06, Pietro Peterlongo @.***> a écrit :

mmh, actually reading about samp https://developer.mozilla.org/en-US/docs/Web/HTML/Element/samp?retiredLocale=it I see it is supposed to be an inline element. I guess we could skip the samp and wrap only in pre?

— Reply to this email directly, view it on GitHub https://github.com/pietroppeter/nimib/pull/185#issuecomment-1455662291, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANDHQOCL37YT63EVIJXV3K3W2WLJ7ANCNFSM6AAAAAAVQJJBWY . You are receiving this because you authored the thread.Message ID: @.***>

[HugoGranstrom]

I actually never noticed this issue about the samp.

It's because it hasn't been a problem until now that water.css has added a background to the samp. We just couldn't see that it wasn't a block before.

mmh, actually reading about samp I see it is supposed to be an inline element. I guess we could skip the samp and wrap only in pre?

If we want to have a background color (that is what water.css added), we will have to put the background on the pre tag and not the samp tag. So we have basically two options:

1. Keep samp but override water.css's styles to the default values. Add background to .nb-output class.
2. Remove samp and just use pre. Add background to .nb-output class.

If it works without the samp I think we should go with option 2.

I am no CSS expert, but I guess we can wrap output both by samp and pre ?

That is what we are currently doing. But as water.css only styles the samp, it looks ugly because it is an inline element. Changing it to display: block works, but at this point we might just as well do this properly and remove the samp.

This is the water.css's CSS for samp:

samp {
  background: #efefef;
  background: var(--background);
  color: #000;
  color: var(--code);
  padding: 2.5px 5px;
  border-radius: 6px;
  font-size: 1em;
}

I would say that we take these attributes and inserts them into our .nb-output class defined in themes.nim in the variable nbStyle.

What do you both think?

[pietroppeter]

Yep option 2 has my vote too (if it works). Thanks for the explanation Hugo. To be honest not sure if we want to add a background or we keep it white. Screenshots with options would help pick.

[HugoGranstrom]

With dark background:

With same background:

With lighter background:

With no background:

[HugoGranstrom]

And a conceptual one with borders around the code+output block and labels:

[HugoGranstrom]

Compared to the others, I actually think the current style without any background looks pretty bad :sweat_smile:

[pietroppeter]

I don't know. What I am not too fond about the versions with background is that output and code seem two separate elements.

An old issue which is relevant is #65. There I linked a deep note example where the link between output and code is made visible through a light border.

[HugoGranstrom]

I don't know. What I am not too fond about the versions with background is that output and code seem two separate elements.

I agree with that. Hence my conceptual suggestion.

An old issue which is relevant is https://github.com/pietroppeter/nimib/issues/65. There I linked a deep note example where the link between output and code is made visible through a light border.

Ohhh, had completely forgot about that one :o Here's a quick version of it hacked together:

The background color of the code is almost too light now. It's hard to see the difference between the white and gray.

[HugoGranstrom]

Here it is with approximately the same background color as deepnotes: And here with the same border-color: The padding can still be improved though, it's a bit squeezed right now:

[HugoGranstrom]

@dlesnoff sorry for going off-topic and discussing other stuff here as well. For this PR I think it would suffice if you remove the samp tag from the output (and the style we added for it) and then we'll merge this and continue with redesigning it in a separate PR.

[dlesnoff]

In which file do you add samp to the output?

Le jeu. 9 mars 2023, 07:44, Hugo Granström @.***> a écrit :

@dlesnoff https://github.com/dlesnoff sorry for going off-topic and discussing other stuff here as well. For this PR I think it would suffice if you remove the samp tag from the output (and the style we added for it) and then we'll merge this and continue with redesigning it in a separate PR.

— Reply to this email directly, view it on GitHub https://github.com/pietroppeter/nimib/pull/185#issuecomment-1461400014, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANDHQOE6YVD23CHKPY2VBUTW3F35HANCNFSM6AAAAAAVQJJBWY . You are receiving this because you were mentioned.Message ID: @.***>

[HugoGranstrom]

Line 20 in renders.nim:

  doc.partials["nbCodeOutput"] = """{{#output}}<pre class="nb-output"><samp>{{output}}</samp></pre>{{/output}}"""

[dlesnoff]

I made the changes. I really like the one with a thick border, but it would help to see in context: what if the code is particularly large (and the border goes off-screen)? what if there are numerous code blocks?

[HugoGranstrom]

Ah right, we have to fix the tests now that we have changed the produced HTML. In tests/trender.nim you should remove the samp tags from the expected output of the tests that fail. Run nimble test locally to run the tests and check if it works

[HugoGranstrom]

And yes we need view the redesigned code blocks in a larger context before deciding the final design. I think something in-between the light and dark borders would be nice.

[pietroppeter]

thanks @dlesnoff for working on this and sorry for all the noise in the discussion!

I am fine with the current changes but noticed a couple of things:

• is this PR really fixing #184 ? I think it is fixing code output block (which was discussed in #184) but not appearance of inline code (at the moment). I am fine with merging this and having a separate PR to work on padding of inline code blocks (also removal of normalize.css as far as I understand is out of scope here).
• I noticed in netlify preview a double horizontal rule before the footer (see screenshot below). not sure if this is due to this PR (it is not an issue present in currently deploy site), it might be because of water.css version 2 fix? it is weird

double hr:

[HugoGranstrom]

We updated Water.css precisely because it solved #184 and reduced the padding of inline code blocks? 😛 In combination with removing normalize.css that is, but seems like we haven't done that here. I thought we did it, must be misremembering 🤔 So that's another thing that has to be done in this PR.

[HugoGranstrom]

So the remaining things TODO in this PR are:

• [x] Remove normalize.css in themes.nim
• [x] Fix tests in tests/trenders.nim

[dlesnoff]

sorry for going off-topic and discussing other stuff here as well.

I do not mind. It seemed to me related to the PR. I was preparing a presentation at a research seminar, so I was not very available this week, that is the reason why I was not very responsive.

[HugoGranstrom]

Thanks a lot, it looks good to me now and the preview looks nice again :D

[dlesnoff]

Do we really want to remove normalize.css ? Code blocks look too small !

[dlesnoff]

Before: After:

[HugoGranstrom]

Then we increase the size of the code specifically for our code blocks instead by creating a class for them. If we keep normalize.css it will make the inline code too big again. And we can't really control the style of the inline code as it is produced by the markdown parser.

[pietroppeter]

I agree on removing normalize css and separately manage size of code. Btw I think the before is too big and agree that after is somewhat too small. I would target code similar in size with normal text. In particular code block should have the same size of inline code and you should not be able to tell the difference of size between inline code and normal text.

[HugoGranstrom]

In particular code block should have the same size of inline code and you should not be able to tell the difference of size between inline code and normal text.

I don't care if code blocks and inline code have the same size. I care about them both looking good. If it happens to be the same size, nice, but if not I don't see why we would limit ourselves to this constraint. We can't really increase the font-size of the inline code, then we would get the overlap again. So my vote is on only increasing the font-size of the code blocks but let the inline code be.

[dlesnoff]

J'ai testé:

code {
  font-size: 1em;
}

et

code {
  font-size: 2em;
}

ainsi qu'inherit. Quelque chose comme 1.3 ou 1.4em peut faire l'affaire, mais ça modifie à la fois le code inline et le code. Effectivement, je préfererais que le code soit modifié indépendamment du code inline qui devrait toujours avoir la même taille que le texte.

[HugoGranstrom]

Thank god for Google Translate :sweat_smile: (easy mistake to do when you forget to context switch :))

Indeed, we should add a new class that we put on the <code> tags in code blocks. And then change the size only for that class.

[dlesnoff]

My train arrived at destination at that time, sorry for the language mixup. I am copy-pasting the original message above with the correct translation (and a few addition):

Previous message

I have tested the following css snippets:

code {
  font-size: 1em;
}

and

code {
  font-size: 2em;
}

as well as inherit as a value of font-size attribute. Something like 1.3 or 1.4em might do apart that it modifies both inline code and code blocks. Indeed, I would prefer that code blocks are modified independently from inline code. Inline code should always have the same size as the text.

What is the name of the partial corresponding to code blocks ?

[HugoGranstrom]

No worries :)

It's on the line above the partial of the code output if I recall correctly.

[pietroppeter]

Btw we should also remove the <hr> inside the footer since this version of water.css adds a top border to the footer element (hence why the double line at the end)

[HugoGranstrom]

The normal text and the code uses different fonts with slightly different sizes. So even if both a 1em now, they don't appear to be as big (code is smaller). For me, they appear roughly equal when code size is set to 1.2em. Are you seeing the same on your screens or does it differ between devices? (I'm on Linux)

[dlesnoff]

It differs between two Linux devices. I don't see the same on my PC (Archlinux) and my laptop (Ubuntu). Using Firefox on both!

[HugoGranstrom]

😅

Might the screen size play into it perhaps? Maybe the normal text is defined in pixels while the code is in relative units. I haven't been able to find the text size of the normal text in the inspector :/

[dlesnoff]

No wait. I pushed the modifications on my blog online, and now the code appears too small on my Archlinux device too. I think I had a confusion between versions of nimib due to a problem in nimble. It seems like the current version of nimble does not select the latest installed version by default, this is weird.

[dlesnoff]

Nimble install does not install my working directory of nimib.

[HugoGranstrom]

Are you using nimble v0.14 perhaps? They changed so much about local installations, I'll stay on 0.13 as long as I can.

[dlesnoff]

I used nimble install nimib instead of nimble install. Yes I use nimble v0.14.

[HugoGranstrom]

I think the preview looks good now, the inline code doesn't overflow and the code blocks looks the same size as the normal text on both my laptop and phone :rocket:

[dlesnoff]

The inline code does overflow for me for some reason. I wonder if nimble install only installs main/master git branch. This would be weird.

[HugoGranstrom]

I wonder if nimble install only installs main/master git branch. This would be weird.

I wouldn't be suprised :P I guess you could do something like nimble install https://github.com/dlesnoff/nimib@updateWater if you want to sanity check that you are using the correct version (and do a nimble remove nimib first to remove all old ones)

[pietroppeter]

This is what I see from iOS/Safari looking at the preview:

Text, in-line code and code black seem they have 3 different sizes, so it does not come out great 🤨 not sure how to help on this though

[pietroppeter]

this is what I see on a desktop windows with Chrome browser:

in this case I do not perceive a big difference between text and code block. inline code keeps being smaller than the two.

[HugoGranstrom]

Text, in-line code and code black seem they have 3 different sizes, so it does not come out great raised_eyebrow not sure how to help on this though

Huh, yeah that doesn't look too good. The inline code is fine, we can't increase its size after all without overflowing so there is nothing to do about it. If the font-size increase, the box around it increases so it's impossible for the inline code to be as big as the normal text. But the code blocks are pretty big/the normal text is small.

I have done some checking on Firefox on my laptop and it seems the normal text gets the default text size 16px while we set the code size to 1.2em. So Safari on IOS might have some other default text size that makes them look different. So we could try and set our own default text size (on body?) to 1em or something so we know that we are working in the same units for all texts.

[HugoGranstrom]

Agreed, and ideally someone who can reproduce the different font sizes should be the one to implement that PR so one doesn't have to wait for someone else just to know if it works or not.