AkkomaGang/akkoma-fe
5
13
Fork 64
Code Issues 81 Pull requests 19 Projects Releases 1 Wiki Activity

WIP: Improve style for quoted text in RichContent #412

Draft
ilja wants to merge 5 commits from ilja/akkoma-fe:improve_visual_style_for_quoted_text into develop
pull from: ilja/akkoma-fe:improve_visual_style_for_quoted_text
merge into: AkkomaGang:develop
Conversation 3 Commits 5 Files changed 1 +12 -2
1 changed files with 12 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
src/components/rich_content/rich_content.scss
View file

@ -1,7 +1,17 @@
@import './../../_variables.scss';
.RichContent {
blockquote {
margin: 0.2em 0 0.2em 2em;
font-style: italic;
margin: 0 0 1em 0.2em;
padding: 0 0 0 1em;
ilja marked this conversation as resolved
Oneric commented 2024-08-25 00:46:27 +00:00
Review
Copy link

suggestion, not a blocker: i’ve also commonly seen text colour being a bit more muted for quoted sections. We could achieve this with something like this:

color: color-mix(in srgb-linear, var(--text), var(--bg) 50%);

or introduce a new style property for quoted text colour. The former may be undesirable in some themes whose text has a poor contrast by default.

suggestion, not a blocker: i’ve also commonly seen text colour being a bit more muted for quoted sections. We could achieve this with something like this: ```css color: color-mix(in srgb-linear, var(--text), var(--bg) 50%); ``` or introduce a new style property for quoted text colour. The former may be undesirable in some themes whose text has a poor contrast by default.
👍 1
floatingghost commented 2024-08-25 09:07:49 +00:00
Review
Copy link

yeah i think i agree, muted quotations seems to be the norm

yeah i think i agree, muted quotations seems to be the norm
👍 1
ilja commented 2024-10-12 13:53:43 +00:00
Review
Copy link

Is fixed for making it faint. I did the same way as how making text faint is done in other locations. It's the same as e.g. the timestamp right-top of the status.

When making a theme for akkoma-fe, there's a check for how readable things remain. I'm unsure how that exactly works, but by using the same way of doing this, i'm assuming this is also taken into account for the theming engine.

There's still some weirdness with space, but that was already a problem. I'll see if I can figure out what's going on there.

Here's how it looks now, and i'm taking the opportunity to show how nested quotes are also nicely working ^^

image

And how it is without these changes

image

Is fixed for making it faint. I did the same way as how making text faint is done in other locations. It's the same as e.g. the timestamp right-top of the status. When making a theme for akkoma-fe, there's a check for how readable things remain. I'm unsure how that exactly works, but by using the same way of doing this, i'm assuming this is also taken into account for the theming engine. There's still some weirdness with space, but that was already a problem. I'll see if I can figure out what's going on there. Here's how it looks now, and i'm taking the opportunity to show how nested quotes are also nicely working ^^ ![image](/attachments/9dbc4dbb-1732-43e4-9a06-8d743fa91d49) And how it is without these changes ![image](/attachments/b4a5c7cd-2aab-4652-b783-8f8523c6f1cc)
image.png
38 KiB
image.png
35 KiB
ilja commented 2024-10-12 15:55:23 +00:00
Review
Copy link

The comment about the space between a paragraph and blockquote is now also fixed 🎉

image

The reason is; p-tags only have margin at the bottom. Blockquotes are block elements on their own, but the margin was different. This gave this problem. See screenshots

Here we check the p-tag. We see there's only a margin at the bottom. That margin is 1em and is what's pushing the next block 1em down.

image

But when we check the blockquote, we see it has margins all around. All are 0.2em, so it only pushes the block below 0.2em down. Between it and the block above, the space is 1em, because there the p-tag has it's 1em margin.

image

Other block elements I can think of who may have similar problems are code, ul and ol-tags. I checked those, and visualy they seem OK. Checking with developer tools (F12 in browseer), it seems they have 1em margin bottom and top, always. We could nitpick wether those should be better to have 0 as last-child, and if they need the margin-top, but if someone wants that, I prefer to have that discussion somewhere else and keep this MR to the blockquotes. If we have that discussion, we should start by checking what elements we allow through the scrubber.

That leaves the problem of the blockqoute looking weird when it comes after a mention, I'll see if I can figure that one out too.

The comment about the space between a paragraph and blockquote is now also fixed 🎉 ![image](/attachments/4ecfae02-4ac7-48dc-a044-ae01e84047b1) The reason is; p-tags only have margin at the bottom. Blockquotes are block elements on their own, but the margin was different. This gave this problem. See screenshots Here we check the p-tag. We see there's only a margin at the bottom. That margin is 1em and is what's pushing the next block 1em down. ![image](/attachments/eea19f76-16d4-4a0c-9f94-c9209046db9b) But when we check the blockquote, we see it has margins all around. All are 0.2em, so it only pushes the block below 0.2em down. Between it and the block above, the space is 1em, because there the p-tag has it's 1em margin. ![image](/attachments/a9afec22-7e15-40d2-8479-c7358598cd65) Other block elements I can think of who may have similar problems are code, ul and ol-tags. I checked those, and visualy they seem OK. Checking with developer tools (F12 in browseer), it seems they have 1em margin bottom and top, always. We could nitpick wether those should be better to have 0 as last-child, and if they need the margin-top, but if someone wants that, I prefer to have that discussion somewhere else and keep this MR to the blockquotes. If we have that discussion, we should start by checking what elements we allow through the scrubber. That leaves the problem of the blockqoute looking weird when it comes after a mention, I'll see if I can figure that one out too.
image.png
20 KiB
image.png
22 KiB
image.png
47 KiB
ilja commented 2024-10-12 16:17:02 +00:00
Review
Copy link

That last problem is also fixed with #347 already. Is there a reason why #347 is still open? Because I think it makes more sense to merge that first. After that I can fix merge conflix on this PR, and then we'll have nice looking quote posts.

That last problem is also fixed with https://akkoma.dev/AkkomaGang/akkoma-fe/pulls/347 already. Is there a reason why https://akkoma.dev/AkkomaGang/akkoma-fe/pulls/347 is still open? Because I think it makes more sense to merge that first. After that I can fix merge conflix on this PR, and then we'll have nice looking quote posts.
ilja commented 2024-10-12 16:24:45 +00:00
Review
Copy link

hm 🤔 I do wonder why that

  .text {
    & > * {
      white-space: pre-wrap;
    }

is for. When I just remove it, it also works. AND when I remove it, codeblocks work better imo. The change in #347 makes the lines "wrap". By removing it, we get a scrollbar.

This is with removing the line

image

This is with the fix from #347

image

I'll make the commit as the first example (where we have the scroll bar) and leave it up to others to decide what they like best.

hm 🤔 I do wonder why that ``` .text { & > * { white-space: pre-wrap; } ``` is for. When I just remove it, it also works. AND when I remove it, codeblocks work better imo. The change in #347 makes the lines "wrap". By removing it, we get a scrollbar. This is with removing the line ![image](/attachments/8c5d13a0-2ec3-4699-8311-3d630545c9db) This is with the fix from #347 ![image](/attachments/a97b1d22-1811-4343-b13d-2d533896cea8) I'll make the commit as the first example (where we have the scroll bar) and leave it up to others to decide what they like best.
image.png
18 KiB
image.png
18 KiB
ilja commented 2024-10-12 16:31:49 +00:00
Review
Copy link

Pushed, I think it's good now. Here's a last screenshot showing blockquotes and different spacings.

image

Pushed, I think it's good now. Here's a last screenshot showing blockquotes and different spacings. ![image](/attachments/258eaefd-f2c5-465e-b874-347231e445c0)
image.png
113 KiB
ilja commented 2024-10-12 16:48:18 +00:00
Review
Copy link

Seems it was an akkoma regression when trying to get mfm to work. Before that, the code I now removed was also just not there. I'm unsure why it was added. $[spin something] seems to still work for example.

@nbsp Do you happen to remember what this white-space: pre-wrap was for (just to be sure we're not breaking other things)? In src/components/status_body/status_body.scss

  .text {
    & > * {
      white-space: pre-wrap;
    }

commit: 931ed3d3c7 (diff-4200029adfe97f22b2316110f9e4adafca58f990)

pr: #29

EDIT: Nevermind, I think I found it. E.g. $[spin 🥴], the whitespace needs to be preserved. Withoutthis line, it collapses to one whitespace. This isn't purely an MFM thing, but I understand that it was added to get better MFM integration.

Seems it was an akkoma regression when trying to get mfm to work. Before that, the code I now removed was also just not there. I'm unsure why it was added. `$[spin something]` seems to still work for example. @nbsp Do you happen to remember what this `white-space: pre-wrap` was for (just to be sure we're not breaking other things)? In src/components/status_body/status_body.scss ``` .text { & > * { white-space: pre-wrap; } ``` commit: https://akkoma.dev/AkkomaGang/akkoma-fe/commit/931ed3d3c77293e1d6087c039e2a613962b00781#diff-4200029adfe97f22b2316110f9e4adafca58f990 pr: https://akkoma.dev/AkkomaGang/akkoma-fe/pulls/29 **EDIT:** Nevermind, I think I found it. E.g. `$[spin 🥴]`, the whitespace needs to be preserved. Withoutthis line, it collapses to one whitespace. This isn't purely an MFM thing, but I understand that it was added to get better MFM integration.
border-width: 0 0 0 0.3em;
border-style: solid;
color: $fallback--faint;
color: var(--faint, $fallback--faint);
}
blockquote:last-child {
margin-bottom: 0;
}
pre {