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
ilja commented 2024-08-21 16:09:02 +00:00
Contributor
Copy link

Previously quoted text (e.g. in Markdown > Some text) was italic
When two different quotes were made, there was no distinction between the two, making it look like one quote
This is confusing

Now we have a vertical line in front of the quote
When two different pieces of text are quoted, it is now clear because the lines are separated
This vertical line is a typical way of visualising quoted text, so it should be easy to understand what it is

Examples:

  • The italic is how it is before this PR. There are two quotes, but this is unclear from the view.
  • Another is the same post, but with the current PR. It is clear what the two different quotes are (and imo more pretty).
  • I also have an example showing that multiple lines are still possible in one quote. (Don't forget, to have a newline with the Markdown option, you need two spaces at the end of the previous line. This is true when not using quotes, but just as true with using quotes.)
Previously quoted text (e.g. in Markdown `> Some text`) was italic When two different quotes were made, there was no distinction between the two, making it look like one quote This is confusing Now we have a vertical line in front of the quote When two different pieces of text are quoted, it is now clear because the lines are separated This vertical line is a typical way of visualising quoted text, so it should be easy to understand what it is Examples: * The italic is how it is before this PR. There are two quotes, but this is unclear from the view. * Another is the same post, but with the current PR. It is clear what the two different quotes are (and imo more pretty). * I also have an example showing that multiple lines are still possible in one quote. (Don't forget, to have a newline with the Markdown option, you need two spaces at the end of the previous line. This is true when not using quotes, but just as true with using quotes.)
Screenshot_20240821_180337.png
14 KiB
Screenshot_20240821_174756.png
30 KiB
Screenshot_20240821_174723.png
37 KiB
Screenshot_20240821_180337.png Screenshot_20240821_174756.png Screenshot_20240821_174723.png
ilja added 1 commit 2024-08-21 16:09:02 +00:00
Improve style for quoted text in RichContent
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
51a51fe6b8 
Previously quoted text (e.g. in Markdown `> Some text`) was italic
When two different quotes were made, there was no destinction between the two, making it look like one quote
This is confusing

Now we have a vertical line in front of the quote
When two different pieces of text are quoted, it is now clear because the lines are separated
This vertical line is a typical way of visualising quoted text, so it should be easy to understand what it is
ilja commented 2024-08-21 16:29:43 +00:00
Author
Contributor
Copy link

One problem I see with this is when reparsing MFM from *key instances... See https://ilja.space/notice/Al8CNka0RS1GrnMnrc

Consider the following as input for MFM:

> Some quoted line
a new line

Akkoma will consider this part of the same quote, *key will only consider the first line as a quote, not the second one.

Before this PR the same thing is true, but bc the quotes are basically just italics, it may be less obvious.

One problem I see with this is when reparsing MFM from *key instances... See https://ilja.space/notice/Al8CNka0RS1GrnMnrc Consider the following as input for MFM: ``` > Some quoted line a new line ``` Akkoma will consider this part of the same quote, *key will only consider the first line as a quote, not the second one. Before this PR the same thing is true, but bc the quotes are basically just italics, it may be less obvious.
Oneric approved these changes 2024-08-25 00:53:20 +00:00
Dismissed
Oneric left a comment
Member
Copy link

I don’t think the pre-existing MFM inportability is a blocker; apart from italic quotes wree already indented before so this wasn’ŧ really better before.

Note there’s also a pre-existing vertical spacing issue; see: #347. It still exists but isn’t introduced by the changes here. The apparently relevant whitespace classes are beyond my CSS knowledge; if you know more and can fix this properly great, otherwise a quick hack to paper over it though is adjusting margns to margin: 0.2em 0 1.0em 0.2em

I don’t think the pre-existing MFM inportability is a blocker; apart from italic quotes wree already indented before so this wasn’ŧ really better before. Note there’s also a pre-existing vertical spacing issue; see: #347. It still exists but isn’t introduced by the changes here. The apparently relevant whitespace classes are beyond my CSS knowledge; if you know more and can fix this properly great, otherwise a quick hack to paper over it though is adjusting margns to `margin: 0.2em 0 1.0em 0.2em`
src/components/rich_content/rich_content.scss
@ -5,0 +3,4 @@
margin: 0.2em 0 0.2em 0.2em;
padding: 0 0 0 1em;
border-width: 0 0 0 0.3em;
border-style: solid;
Oneric commented 2024-08-25 00:46:27 +00:00
Member
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
Owner
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
Author
Contributor
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
Author
Contributor
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
Author
Contributor
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
Author
Contributor
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
Author
Contributor
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
Author
Contributor
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.
ilja marked this conversation as resolved
ilja added 1 commit 2024-10-12 13:45:23 +00:00
Make text faint for blockquotes
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
a9367d444a 
In code review it was decided that faint text for blockquotes should be used.
I copied how it was done in other places to faint text.

When making a theme for *oma-fe, there's a check for how readable things remain.
I'm unsure how that exactly works, but timestamps for a status is also faint,
so by using the same way of doing this, this should also be taken into account
for the theming engine.
ilja added 1 commit 2024-10-12 15:34:50 +00:00
Fix spacing between paragraph and blockquote
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
fa058ca093 
Blockquotes are blocks, so not wrapped in an extra p-tag.

In statusses this gave an unfortunate result that the margins were different.
A p-tag has a bottom margin of 1em. Blockquotes had 0.2em top and bottom.

So under a paragraph there was 1em space, but under the blockquote, there was only 0.2em space.

The last p-tag has 0 margin at the bottom.

This commit basically does the same thing for blockquotes now, making it more consistent.

One difference is that the blockquote has a left margin of 0.2em because a little "jump"
in makes it look a bit better imo.
ilja added 1 commit 2024-10-12 16:29:14 +00:00
Remove pre-wrap from status_body
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
01164fc520 
For some strange reason, after a mention a quote would be double as high as it should.

Removing this "pre-wrap" seems to fix this. I'm not sure what it was exactly for, but I don't see anything break.

The code blocks now don't wrap any more, but show a scroll bar, which imo is better for a code block.
Oneric approved these changes 2024-10-13 22:47:06 +00:00
Oneric left a comment
Member
Copy link

Seems to work fine in testing; thanks!

Nice find re --faint.
I’m not really qualified to judge the whitespace: pre-wrap stuff, but the explanation sounds convincing and i didn’t encounter any obvious issues so far. With the scroll bar disappearing by default in Firefox at least, the presence of overflowed content might not be obvious if the overflow occurs within whitespace or on a character boundary, but this is a matter for a different PR imho (e.g. distinguishing code blocks with a different background colour, which might also necessitate a different text color for readability unless we already have something fitting?)

Seems to work fine in testing; thanks! Nice find re `--faint`. I’m not really qualified to judge the `whitespace: pre-wrap` stuff, but the explanation sounds convincing and i didn’t encounter any obvious issues so far. With the scroll bar disappearing by default in Firefox at least, the presence of overflowed content might not be obvious if the overflow occurs within whitespace or on a character boundary, but this is a matter for a different PR imho *(e.g. distinguishing code blocks with a different background colour, which might also necessitate a different text color for readability unless we already have something fitting?)*
👍 1
ilja commented 2024-10-16 16:13:51 +00:00
Author
Contributor
Copy link

heh, something I just noticed.

Here's a post with a quote

image

Here's the same post when I select it in a thread

image

Note that the second one doesn't look fainted any more. And indeed, I did it the same as the timestamp and that one doesn't look faint any more either.

Honestly, I like it like this. It's fainted in "normal" situations, but when you select it, it's more readable. I see that e.g. the globe still looks faint, so I could probably change it to keep the quote look faint too, but personally I think it's better as it is now.

heh, something I just noticed. Here's a post with a quote ![image](/attachments/afe11419-477d-4a55-bcc7-dd5c1da08065) Here's the same post when I select it in a thread ![image](/attachments/80600377-81c3-4ae1-adb0-ab9ed4a2a8f4) Note that the second one doesn't look fainted any more. And indeed, I did it the same as the timestamp and that one doesn't look faint any more either. Honestly, I like it like this. It's fainted in "normal" situations, but when you select it, it's more readable. I see that e.g. the globe still looks faint, so I could probably change it to keep the quote look faint too, but personally I think it's better as it is now.
image.png
48 KiB
image.png
170 KiB
ilja changed title from Improve style for quoted text in RichContent to WIP: Improve style for quoted text in RichContent 2024-10-20 09:23:36 +00:00
ilja added 1 commit 2024-10-20 09:56:22 +00:00
Revert "Remove pre-wrap from status_body"
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
7f08fe9dc4 
This reverts commit 01164fc520.
ilja commented 2024-10-20 10:14:37 +00:00
Author
Contributor
Copy link

Seems to work fine in testing; thanks!

Nice find re --faint.
I’m not really qualified to judge the whitespace: pre-wrap stuff, but the explanation sounds convincing and i didn’t encounter any obvious issues so far. With the scroll bar disappearing by default in Firefox at least, the presence of overflowed content might not be obvious if the overflow occurs within whitespace or on a character boundary, but this is a matter for a different PR imho (e.g. distinguishing code blocks with a different background colour, which might also necessitate a different text color for readability unless we already have something fitting?)

I discovered why the whitespace: pre-wrap is there. Without it, whitespaces will collapse into one space. See screenshots. For some reason, keeping the spaces only happens with Markdown and MFM (who also uses Markdown). I'm assuming we should keep the white space for MFM compatibility.

With whitespace: pre-wrap we see on the first line that spaces are preserved. But it makes blockquotes slightly weird after a mention and a space:

image

Without whitespace: pre-wrap we see on the first line that spaces are not preserved. The weird blockquote thing is fixed:

image

My conclusions:

  • We probably want to keep the spaces for MFM compatibility
  • The weird thing with the blockquotes only happens right after a mention followed by a space. I don't know why.
  • It's not actually a regression, but it is more obvious due to the way quotes are now presented.
  • If we want to properly fix how things look, I think it should be done in a separate PR, starting from an example post who contains all possibilities (mentions, block quotes, code, overflows...), so we can see at one glance what's happening.
  • This is going way deeper than I originally wanted to do with this PR.

What now:

I reverted that "fix" now. This means that spaces are preserved again (good), but in some specific occasions the blockquotes will have an empty space above them (less good).

The current improvements are

  1. Instead of using italics, quotes are now shown with a line on the left + the text is somewhat fainter than other text
  2. When the post is explicitly selected, the line on the left is still there, but the text is not fainted any more so that it is clearly readable.

image

The bad thing is the weirdness with block quotes after a mention. I'm not very happy with how it is now, but I'm also not sure I want to spend more time on this at this moment since this was meant to be a quick fix...

> Seems to work fine in testing; thanks! > > Nice find re `--faint`. > I’m not really qualified to judge the `whitespace: pre-wrap` stuff, but the explanation sounds convincing and i didn’t encounter any obvious issues so far. With the scroll bar disappearing by default in Firefox at least, the presence of overflowed content might not be obvious if the overflow occurs within whitespace or on a character boundary, but this is a matter for a different PR imho *(e.g. distinguishing code blocks with a different background colour, which might also necessitate a different text color for readability unless we already have something fitting?)* > I discovered why the `whitespace: pre-wrap` is there. Without it, whitespaces will collapse into one space. See screenshots. For some reason, keeping the spaces only happens with Markdown and MFM (who also uses Markdown). I'm assuming we should keep the white space for MFM compatibility. With `whitespace: pre-wrap` we see on the first line that spaces are preserved. But it makes blockquotes slightly weird after a mention and a space: ![image](/attachments/28d62726-7c84-47e0-9f55-f6be8996edda) Without `whitespace: pre-wrap` we see on the first line that spaces are not preserved. The weird blockquote thing is fixed: ![image](/attachments/694e6dcf-3ae6-43eb-aab8-090780548763) --- My conclusions: - We probably want to keep the spaces for MFM compatibility - The weird thing with the blockquotes only happens right after a mention followed by a space. I don't know why. - It's not actually a regression, but it is more obvious due to the way quotes are now presented. - If we want to properly fix how things look, I think it should be done in a separate PR, starting from an example post who contains all possibilities (mentions, block quotes, code, overflows...), so we can see at one glance what's happening. - This is going way deeper than I originally wanted to do with this PR. What now: I reverted that "fix" now. This means that spaces are preserved again (good), but in some specific occasions the blockquotes will have an empty space above them (less good). The current improvements are 1. Instead of using italics, quotes are now shown with a line on the left + the text is somewhat fainter than other text 2. When the post is explicitly selected, the line on the left is still there, but the text is not fainted any more so that it is clearly readable. ![image](/attachments/7da18543-7f86-4244-9470-3c6f306ce2f8) The bad thing is the weirdness with block quotes after a mention. I'm not very happy with how it is now, but I'm also not sure I want to spend more time on this at this moment since this was meant to be a quick fix...
image.png
37 KiB
image.png
29 KiB
image.png
12 KiB
image.png
36 KiB
image.png
12 KiB
image.png image.png
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u improve_visual_style_for_quoted_text:ilja-improve_visual_style_for_quoted_text
git checkout ilja-improve_visual_style_for_quoted_text
Sign in to join this conversation.
Reviewers
No reviewers
Oneric
Labels
Clear labels   
a11y

Issues and PRs about accessibility

  
Bug

Applies to issues describing bugs

  
Bug fix

Pull requests that squash bugs

  
Critical Priority

   
Documentation

  
Feature

Pull requests that implement new features

  
Feature request

Issues that request new features

  
Held for next release cycle

PR reviewed during release candidate phase, approved and held as PR

  
High Priority

   
Low Priority

   
Medium Priority

   
Minor change

For minor changes, ie CSS alignment changes

  
Translation/Locale

For changes related to translations

  
WIP

Work in progress

No labels
a11y
Bug
Bug fix
Critical Priority
Documentation
Feature
Feature request
Held for next release cycle
High Priority
Low Priority
Medium Priority
Minor change
Translation/Locale
WIP
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma-fe#412
No description provided.
Delete branch "ilja/akkoma-fe:improve_visual_style_for_quoted_text"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?