pleroma/pleroma-fe
0
0
Fork 0
Code Issues 343 Pull requests 27 Projects Releases Packages Wiki Activity

Status HTML parsing - better emoji and mentions rendering #2659

Closed
hj wants to merge 0 commits from gitlab-mr-iid-1392 into develop
pull from: gitlab-mr-iid-1392
merge into: pleroma:develop
Conversation 43 Commits - Files changed -
hj commented 2021-06-07 00:28:37 +00:00
Owner
Copy link

This is WIP but I like this much more than previous attempt (replacing in runtime)

User-facing changes:

  • Adds new instance-default (pleroma default: false) setting to move mentions in the beginning of post into separate line
  • Posts from instances that don't put mentions in posts will have mentions in said separate line regardless of setting
  • Mentions now have user-highlights
  • Add new instance-default (pleroma default: false) setting to use new style of mentions that displays them as buttons instead of links which makes user-highlights more visible
  • Fixed #935, greentext now ignores <code> and <blockquote>. It actually now detects level properly (was broken)
  • Greentext now accompanied by cyantext (when line begins with <), uses blue color instead of cyan tho.
  • GIF Emoji now follow same suit as rest of GIF images - obeying the "play-on-hover GIFs" setting. Still no solution for APNGs tho.

Technical details

  • Refactors old tiny_html_processor
    • Now part of html_converter folder.
    • Now called html_line_converter - It converts HTML text into an array of "visual lines", as it did before.
    • No longer processes things as it goes, instead returns an array you can .map() over to process it.
  • Adds new html_tree_converter - It converts HTML text into a tree structure representing tags and text nodes, intended to be used in rendering process.
    • in addition, these new helper functions:
      • getTagName - relatively simple RegExp that extracts name from tag, i.e. <span class="bbbb"> -> span, updated version from tiny post html processor, TODO update original one to use this.
      • getAttrs - similar to previous one except it extracts attributes into a map, i.e. <a class="a" href="b"> -> { class: 'a', href: 'b' }
      • processTextForEmoji - optimized function allowing you to convert string with emoji to an array with chunks of string + whatever you want replacing the emoji.
  • Adds new RichContent component - the main star of the show. It's purpose is to enhance post body/emoji-containing HTML and give us control over it. Currently it does:
    • Collects and counts mentions and tags - all, in the beginning, in the end.
    • Change all mentions (detected via .mentions class) into MentionLink
    • If last line only contains mentions - wraps them in a
      • If there are no mentions in the beginning we consider it a hellthread-style and remove those last mentions.
    • Update all links to have `target="_blank"
    • Update all images to use StillImage component
    • Add emoji by replacing :shortcode: with StillImage component.
      • This finally relieves entity_normalizer from duty of adding emojis, now they are added at component level where they belong, not on data level.
      • For this purpose entity_normalizer now also returns raw_html
    • Emits parseReady event when it's done rendering, event contains information about first, last, all mentions and also tags. (used in StatusBody and Status)
    • Only used within Status component for user name display and for post subject and text
  • Adds new MentionLink component
    • Represents mention link
    • Uses button and opens profile on click, no need to catch clicks and prevent them.
    • Has option for new style where link looks more like a button
    • Adds a "tooltip" that contains full handle (also used for copying)
  • Adds new MentionsLine component
    • Limits the amount of mentions displayed, show the "+X more"
  • Adds new StatusBody component
    • Separates handling of post subject/text into a separate component.
      • Structure now is like this: Status -> StatusContent -> StatusBody
      • StatusBody is useful when you want to render post content but don't want to render attachments
      • Also does small improvements to how things are displayed (or not displayed) for future enhancements (compact notifications)
  • Improves Status component: (parts of what's described also implemented in StatusBody where applicable)
    • Adds new "show mentions on separate line" option. This removes mentions from post body and puts them on separate line (see below). This option is forcefully switched off in notifications since it's hard to tell whether post someone liked is an OP or a reply.
    • Moves "Replies" line at the very bottom, below the attachments:
      image
    • Adds "Mentions" next to "Reply to"
      image
      • When using "Show mentions on separate line" it shows all non-duplicate mentions (i.e. status.attentions) mentions
      • Otherwise it only shows mentions that weren't present in post body to begin with (i.e. coming from servers/clients that do not put mentions in post body)
    • When not using "Show mentions on separate line" it leaves mentions where they were in same order and amount (i.e. including duplicates) as they were and wraps them in a MentionsLine:
      image

All of this is live on https://shigusegubu.club/ while i'm testing it

Curent bugs:

  • Some links don't open in _blank, should probably change target in processor instead of catching clicks like we do now
  • "Mentions" row has underline on hover and hand cursor even though it doesn't do anything
  • "Mentions" row is different than reply row in sttus popover
  • "Old place" mentions are somewhat forced, if post didn't have mentions in the beginning it will add them. Need to make html processor stateful and able to return more than just changed tree.
  • This post https://shigusegubu.club/notice/A8BKHGflWmonlmOJjk doesn't have proper spacing between tags for some reason. Probably reverse-processor trimming the strings when it shouldn't.

Regressions:

  • Clicking tags would open them on another instance, need to implement TagLink, most of the stuff for it is already there.
  • If post has an image with width=... height=... those would be ignored as StillImage doesn't understand them. Still, it's a rarity and can be fixed if we ever encounter it. Not like everyone is happy with our current article rendering.
  • Current "tooltip" implementation is very dumb and causes horizontal scroll in notifications column in some cases (chrome + autoscroll extension), worst case scenario we'll make it 0x0px so that it doesn't take space but you still can copy from it when selecting the mention.
This is WIP but I like this much more than previous attempt (replacing in runtime) ## User-facing changes: - Adds new instance-default (pleroma default: false) setting to move mentions in the beginning of post into separate line - Posts from instances that don't put mentions in posts will have mentions in said separate line regardless of setting - Mentions now have user-highlights - Add new instance-default (pleroma default: false) setting to use new style of mentions that displays them as buttons instead of links which makes user-highlights more visible - Fixed #935, greentext now ignores `<code>` and `<blockquote>`. It actually now detects level properly (was broken) - Greentext now accompanied by cyantext (when line begins with `<`), uses blue color instead of cyan tho. - GIF Emoji now follow same suit as rest of GIF images - obeying the "play-on-hover GIFs" setting. Still no solution for APNGs tho. ## Technical details - Refactors old `tiny_html_processor` - Now part of `html_converter` folder. - Now called `html_line_converter` - It converts HTML text into an array of "visual lines", as it did before. - No longer processes things as it goes, instead returns an array you can `.map()` over to process it. - Adds new `html_tree_converter` - It converts HTML text into a tree structure representing tags and text nodes, intended to be used in rendering process. - in addition, these new helper functions: - `getTagName` - relatively simple RegExp that extracts name from tag, i.e. `<span class="bbbb">` -> `span`, updated version from tiny post html processor, TODO update original one to use this. - `getAttrs` - similar to previous one except it extracts attributes into a map, i.e. `<a class="a" href="b">` -> `{ class: 'a', href: 'b' }` - `processTextForEmoji` - optimized function allowing you to convert string with emoji to an array with chunks of string + whatever you want replacing the emoji. - Adds new `RichContent` component - the main star of the show. It's purpose is to enhance post body/emoji-containing HTML and give us control over it. Currently it does: - Collects and counts mentions and tags - all, in the beginning, in the end. - Change all mentions (detected via `.mentions` class) into `MentionLink` - If last line only contains mentions - wraps them in a <span> - If there are no mentions in the beginning we consider it a hellthread-style and remove those last mentions. - Update all links to have `target="_blank" - Update all images to use `StillImage` component - Add emoji by replacing `:shortcode:` with `StillImage` component. - This finally relieves `entity_normalizer` from duty of adding emojis, now they are added at component level where they belong, not on data level. - For this purpose `entity_normalizer` now also returns `raw_html` - Emits `parseReady` event when it's done rendering, event contains information about first, last, all mentions and also tags. (used in StatusBody and Status) - Only used within `Status` component for user name display and for post subject and text - Adds new `MentionLink` component - Represents mention link - Uses button and opens profile on click, no need to catch clicks and prevent them. - Has option for new style where link looks more like a button - Adds a "tooltip" that contains full handle (also used for copying) - Adds new `MentionsLine` component - Limits the amount of mentions displayed, show the "+X more" - Adds new `StatusBody` component - Separates handling of post subject/text into a separate component. - Structure now is like this: `Status -> StatusContent -> StatusBody` - StatusBody is useful when you want to render post content but don't want to render attachments - Also does small improvements to how things are displayed (or not displayed) for future enhancements (compact notifications) - Improves `Status` component: (parts of what's described also implemented in `StatusBody` where applicable) - Adds new "show mentions on separate line" option. This removes mentions from post body and puts them on separate line (see below). This option is forcefully switched off in notifications since it's hard to tell whether post someone liked is an OP or a reply. - Moves "Replies" line at the very bottom, below the attachments: ![image](/attachments/53fa0924-14e1-4f7d-8ba5-3ec2d505240f) - Adds "Mentions" next to "Reply to" ![image](/attachments/98b16c6f-cd47-4011-bbae-0b8f06c7087b) - When using "Show mentions on separate line" it shows all non-duplicate mentions (i.e. `status.attentions`) mentions - Otherwise it only shows mentions that weren't present in post body to begin with (i.e. coming from servers/clients that do not put mentions in post body) - When not using "Show mentions on separate line" it leaves mentions where they were in same order and amount (i.e. including duplicates) as they were and wraps them in a `MentionsLine`: ![image](/attachments/a253c71d-b662-461c-bb2f-ce9a774583f7) All of this is live on https://shigusegubu.club/ while i'm testing it Curent bugs: - [x] Some links don't open in _blank, should probably change target in processor instead of catching clicks like we do now - [x] "Mentions" row has underline on hover and hand cursor even though it doesn't do anything - [x] "Mentions" row is different than reply row in sttus popover - [x] "Old place" mentions are somewhat forced, if post didn't have mentions in the beginning it will add them. Need to make html processor stateful and able to return more than just changed tree. - [x] This post https://shigusegubu.club/notice/A8BKHGflWmonlmOJjk doesn't have proper spacing between tags for some reason. Probably reverse-processor trimming the strings when it shouldn't. Regressions: - Clicking tags would open them on another instance, need to implement `TagLink`, most of the stuff for it is already there. - If post has an image with `width=... height=...` those would be ignored as `StillImage` doesn't understand them. Still, it's a rarity and can be fixed if we ever encounter it. Not like everyone is happy with our current article rendering. - Current "tooltip" implementation is very dumb and causes horizontal scroll in notifications column in some cases (chrome + autoscroll extension), worst case scenario we'll make it 0x0px so that it doesn't take space but you still can copy from it when selecting the mention.
image.png
6.5 KiB
image.png
24 KiB
image.png
18 KiB
hj commented 2021-06-07 22:04:42 +00:00
Author
Owner
Copy link

Current style of mentions:
image

Tweaked spacings and background a bit. Mentions without highlights now have transparent background, so it blends in with the rest of the post, @ symbol now uses faint color so it's not as standout.

Depending on theme you might not even notice that it uses button styles.
image

After much tweaking I'm happy with how it looks.

Current style of mentions: ![image](/attachments/a18c4260-52de-4c4f-a3a2-fb2a0f6f6dc9) Tweaked spacings and background a bit. Mentions without highlights now have transparent background, so it blends in with the rest of the post, `@` symbol now uses faint color so it's not as standout. Depending on theme you might not even notice that it uses button styles. ![image](/attachments/43a2cc7c-7e7e-49af-aa81-deaf7a5afc0e) After much tweaking I'm happy with how it looks.
image.png
87 KiB
image.png
38 KiB
hj commented 2021-06-08 10:14:39 +00:00
Author
Owner
Copy link

comparison:

vanilla old new old&moved new&moved
image image image image image

There's a slight bug with extra spaces in beginning that I'll fix later.

comparison: | vanilla | old | new | old&moved | new&moved | | ------ | ------ | ------ | ------ | ------ | | ![image](/attachments/e2b16442-3675-42a6-8cca-65c16ada37bf) | ![image](/attachments/88e8d944-e3e8-4b02-af12-cff96c979b49) | ![image](/attachments/85fd1f71-bd11-400e-912b-2511993f84e0) | ![image](/attachments/21387f2d-9ab6-41ae-9105-860e41cc2a58) |![image](/attachments/7f931ea7-d4d0-4b67-ba54-724a48ca8213) | There's a slight bug with extra spaces in beginning that I'll fix later.
image.png
55 KiB
image.png
56 KiB
image.png
57 KiB
image.png
55 KiB
image.png
56 KiB
hj commented 2021-06-12 17:33:38 +00:00
Author
Owner
Copy link

image

![image](/attachments/5a8bf193-1184-47df-a9e6-ddb7b272072c)
image.png
26 KiB
hj commented 2021-06-12 17:42:12 +00:00
Author
Owner
Copy link

eslint --fix result 🤷

eslint --fix result :shrug:
hj commented 2021-06-12 17:42:12 +00:00
Author
Owner
Copy link

unnecessary

unnecessary
hj commented 2021-06-12 17:42:13 +00:00
Author
Owner
Copy link

make if configurable? What everyone thinks?

make if configurable? What everyone thinks?
hj commented 2021-06-12 17:42:13 +00:00
Author
Owner
Copy link
          // Post we want to see is taller than screen so match its top to screen top
```suggestion:-0+0 // Post we want to see is taller than screen so match its top to screen top ```
hj commented 2021-06-12 18:55:39 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2021-06-13 16:16:19 +00:00
Author
Owner
Copy link

There's one bug related to statuses coming from mastodon:

image
It's supposed to be single line

It's probably unfixable until Vue3, to fix it we need to move the mentions line inside <RichContent>, and to do that we need to use slots, but right now we can't use slots because they cause renderloop since we emit an event inside render function (no, moving it to update () is even worse, I tried).

There's one bug related to statuses coming from mastodon: ![image](/attachments/4f28b3d2-a4c4-4d5a-a435-f064649d8dc3) It's supposed to be single line It's probably unfixable until Vue3, to fix it we need to move the mentions line inside `<RichContent>`, and to do that we need to use slots, but right now we can't use slots because they cause renderloop since we emit an event inside render function (no, moving it to `update ()` is even worse, I tried).
image.png
12 KiB
hj commented 2021-06-14 07:33:59 +00:00
Author
Owner
Copy link

i fixed it lol. not so impossible now huh

i fixed it lol. not so impossible now huh
hj commented 2021-06-15 10:26:27 +00:00
Author
Owner
Copy link

another bug found

https://shigusegubu.club/notice/A8J5cIH2pDuYll4jmy
image

another bug found https://shigusegubu.club/notice/A8J5cIH2pDuYll4jmy ![image](/attachments/5ecb848c-bdfa-4074-a418-7a12788f8fa9)
image.png
208 KiB
hj commented 2021-06-15 11:50:03 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2021-06-15 17:20:29 +00:00
Author
Owner
Copy link

more bugs

https://shigusegubu.club/notice/A8Jckvw2XwpWa2yhw8

image
image

more bugs https://shigusegubu.club/notice/A8Jckvw2XwpWa2yhw8 ![image](/attachments/8d3fd2ea-0a0c-4363-80fd-850333360d65) ![image](/attachments/aebd0bb9-780d-4f51-8b94-4d36d33eadcb)
image.png
5.3 KiB
image.png
264 KiB
hj commented 2021-06-15 22:45:09 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2021-06-18 17:14:40 +00:00
Author
Owner
Copy link

image

ah shit, here we go again

![image](/attachments/66d79789-e6f1-4903-b0b9-c083194b8f53) ah shit, here we go again
image.png
15 KiB
hj commented 2021-06-18 18:07:00 +00:00
Author
Owner
Copy link

More bugs

https://shigusegubu.club/notice/A8PyRg4ieZ6fpU1Kzo

Screenshot_20210618_210624image

More bugs https://shigusegubu.club/notice/A8PyRg4ieZ6fpU1Kzo ![Screenshot_20210618_210624](/attachments/507a0e0b-caab-4c03-9c7f-8ffb1ef013c5)![image](/attachments/10a02975-36a9-4b4a-95c7-5a10c9e1a86a)
image.png
1.1 MiB
Screenshot_20210618_210624.png
100 KiB
hj commented 2021-06-18 18:46:11 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2021-06-18 19:18:29 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2021-06-22 11:31:02 +00:00
Author
Owner
Copy link

yet another weirdness:

https://shigusegubu.club/notice/A8XglVEkPoJs62h7Fw

looks like there's a mention that is NOT present in attentions object.

yet another weirdness: https://shigusegubu.club/notice/A8XglVEkPoJs62h7Fw looks like there's a mention that is NOT present in `attentions` object.
shpuld commented 2021-07-19 16:53:43 +00:00
Owner
Copy link

Needs to be properly benchmarked, as status rendering and destruction is already our biggest bottleneck, need to make sure that converting the mentions to vue stuff doesn't add much more overhead

Needs to be properly benchmarked, as status rendering and destruction is already our biggest bottleneck, need to make sure that converting the mentions to vue stuff doesn't add much more overhead
shpuld commented 2021-07-19 16:57:57 +00:00
Owner
Copy link

I'm not sure how I feel about the mentions looking so distinct, looks kinda "heavy" even when the button style is supposed to make it look more subtle

I'm not sure how I feel about the mentions looking so distinct, looks kinda "heavy" even when the button style is supposed to make it look more subtle
shpuld commented 2021-07-19 16:59:05 +00:00
Owner
Copy link

is the option for moving the mentions really something practical or something we want to maintain? I still think it's a bit of a weird thing that further separates what you write from what is displayed

is the option for moving the mentions really something practical or something we want to maintain? I still think it's a bit of a weird thing that further separates what you write from what is displayed
shpuld commented 2021-07-19 17:00:03 +00:00
Owner
Copy link

compare id instead maybe

compare id instead maybe
shpuld commented 2021-07-19 17:01:55 +00:00
Owner
Copy link

I personally don't think it needs to be configurable, but at the same time I didn't think anyone would throw a fit over the attachment thumbnail minimizing back in the day either

I personally don't think it needs to be configurable, but at the same time I didn't think anyone would throw a fit over the attachment thumbnail minimizing back in the day either
shpuld commented 2021-07-19 17:02:55 +00:00
Owner
Copy link

"-sublime" doesn't say anything to me, more descriptive name maybe?

"-sublime" doesn't say anything to me, more descriptive name maybe?
hj commented 2021-07-19 17:15:15 +00:00
Author
Owner
Copy link

after long long long dogfooding i ended up using simple style the most, on top of that styled mentions option gives me the dread that it really shouldn't be an option but rather we should add it to theme engine so that it doesn't have to look like a button specifically.

I think I'll remove the "cool" style altogether, and reintroduce it in the mystical upcoming themes v3

after long long long dogfooding i ended up using simple style the most, on top of that styled mentions option gives me the dread that it really shouldn't be an option but rather we should add it to theme engine so that it doesn't have to look like a button specifically. I think I'll remove the "cool" style altogether, and reintroduce it in the mystical upcoming themes v3
hj commented 2021-07-19 17:19:16 +00:00
Author
Owner
Copy link

after aformentioned dogfooding and also thinking about maintaining it and all of the usecases that I haven't covered, I think it's better to remove this "moving". It saves space overall but isn't exactly all that useful or convenient.

We can remove the option for moving mentions but a lot of code related to it would remain, notably:

  • separate mentions line is useful for posts originating from other services that do not put mentions in post body (was it friendica?)
  • to limit mentions we still need to replace first/last mentions with a MentionsLine
after aformentioned dogfooding and also thinking about maintaining it and all of the usecases that I haven't covered, I think it's better to remove this "moving". It saves space overall but isn't exactly all that useful or convenient. We can remove the option for moving mentions but a lot of code related to it would remain, notably: - separate mentions line is useful for posts originating from other services that do not put mentions in post body (was it friendica?) - to limit mentions we still need to replace first/last mentions with a MentionsLine
hj commented 2021-07-19 17:19:48 +00:00
Author
Owner
Copy link

I'll just remove the whole "cool style" thing and add it later in a better way.

I'll just remove the whole "cool style" thing and add it later in a better way.
hj commented 2021-07-19 17:19:57 +00:00
Author
Owner
Copy link

sure

sure
hj commented 2021-07-19 17:21:15 +00:00
Author
Owner
Copy link

i'll try to do some benchmarking, maybe even in form of a unit-test, as well as (developer) option to use legacy rendering.

i'll try to do some benchmarking, maybe even in form of a unit-test, as well as (developer) option to use legacy rendering.
shpuld commented 2021-07-19 18:34:41 +00:00
Owner
Copy link

fair enough, yeah keeping the code for the features we want still makes sense

fair enough, yeah keeping the code for the features we want still makes sense
hj commented 2021-08-12 13:06:50 +00:00
Author
Owner
Copy link

Performance testing:

This is full-mounting 9001 richcontents/v-html divs saying "@Lain i just landed in l a where are you" in a unit "test" on my machine, obviously would be slower on slower machines.

Test doesn't assert anything, just outputs result into console.

Running unit test in console:

LOG: '9001 items with links handling:'
LOG: 'Mount: 1152ms, destroy: 7ms, avg 0.12798577935784913ms - 0.0007776913676258194ms per item'
LOG: '9001 items without links handling:'
LOG: 'Mount: 258ms, destroy: 5ms, avg 0.02866348183535163ms - 0.0005554938340184424ms per item'
LOG: '9001 items plain v-html:'
LOG: 'Mount: 302ms, destroy: 6ms, avg 0.03355182757471392ms - 0.0006665926008221309ms per item'

Chromium 90.0.4430

9001 items with links handling:
Mount: 1390.7799999578856ms, destroy: 5.655000044498593ms, avg 0.154513942890555ms - 0.0006282635312185971ms per item
9001 items without links handling:
Mount: 318.67499998770654ms, destroy: 2.3900000378489494ms, avg 0.035404399509799636ms - 0.00026552605686578707ms per item
9001 items plain v-html:
Mount: 188.95000003976747ms, destroy: 2.3249999503605068ms, avg 0.020992111991975055ms - 0.00025830462730368924ms per item

Firefox 90.0.0

9001 items with links handling: 
Mount: 5082ms, destroy: 5ms, avg 0.5646039328963448ms - 0.0005554938340184424ms per item 
9001 items without links handling:
Mount: 2248ms, destroy: 3ms, avg 0.2497500277746917ms - 0.00033329630041106545ms per item
9001 items plain v-html:
Mount: 2219ms, destroy: 5ms, avg 0.24652816353738474ms - 0.0005554938340184424ms per item

No idea why firefox is so fucking slow.

On average it seems that there's almost no difference between linkless richcontent and plain v-html. Destroy times are miniuscle across the board. Replacing one mention with MentionLink does cause a slowdown.

A bit more realistic case (300 items with 10 mentions each, i.e. ye olde hellthread, chromium):

300 items with links handling:
Mount: 588.2700000074692ms, destroy: 3.9049999904818833ms, avg 1.9609000000248973ms - 0.013016666634939611ms per item
300 items without links handling:
Mount: 80.83499997155741ms, destroy: 1.2250000145286322ms, avg 0.26944999990519136ms - 0.004083333381762107ms per item
300 items plain v-html:
Mount: 66.73999998020008ms, destroy: 0.3849999629892409ms, avg 0.22246666660066694ms - 0.0012833332099641364ms per item

and same 10-mentions, but 20 items (same browser):

20 items with links handling:
Mount: 61.0300000407733ms, destroy: 0.5600000149570405ms, avg 3.051500002038665ms - 0.028000000747852027ms per item
20 items without links handling:
Mount: 8.83500004420057ms, destroy: 0.2799999783746898ms, avg 0.44175000221002847ms - 0.013999998918734491ms per item
20 items plain v-html:
Mount: 10.869999998249114ms, destroy: 0.26999996043741703ms, avg 0.5434999999124557ms - 0.013499998021870852ms per item

Note that this does not include all the other stuff that comes with Status component.

I'm also not testing custom emoji rendering because we do really want play-on-hover gifs there.

What's your take, @shpuld?

Performance testing: This is full-mounting 9001 richcontents/v-html divs saying "`@Lain i just landed in l a where are you`" in a unit "test" on my machine, obviously would be slower on slower machines. Test doesn't assert anything, just outputs result into console. Running unit test in console: ``` LOG: '9001 items with links handling:' LOG: 'Mount: 1152ms, destroy: 7ms, avg 0.12798577935784913ms - 0.0007776913676258194ms per item' LOG: '9001 items without links handling:' LOG: 'Mount: 258ms, destroy: 5ms, avg 0.02866348183535163ms - 0.0005554938340184424ms per item' LOG: '9001 items plain v-html:' LOG: 'Mount: 302ms, destroy: 6ms, avg 0.03355182757471392ms - 0.0006665926008221309ms per item' ``` Chromium 90.0.4430 ``` 9001 items with links handling: Mount: 1390.7799999578856ms, destroy: 5.655000044498593ms, avg 0.154513942890555ms - 0.0006282635312185971ms per item 9001 items without links handling: Mount: 318.67499998770654ms, destroy: 2.3900000378489494ms, avg 0.035404399509799636ms - 0.00026552605686578707ms per item 9001 items plain v-html: Mount: 188.95000003976747ms, destroy: 2.3249999503605068ms, avg 0.020992111991975055ms - 0.00025830462730368924ms per item ``` Firefox 90.0.0 ``` 9001 items with links handling: Mount: 5082ms, destroy: 5ms, avg 0.5646039328963448ms - 0.0005554938340184424ms per item 9001 items without links handling: Mount: 2248ms, destroy: 3ms, avg 0.2497500277746917ms - 0.00033329630041106545ms per item 9001 items plain v-html: Mount: 2219ms, destroy: 5ms, avg 0.24652816353738474ms - 0.0005554938340184424ms per item ``` No idea why firefox is so fucking slow. On average it seems that there's almost no difference between linkless richcontent and plain v-html. Destroy times are miniuscle across the board. Replacing one mention with MentionLink does cause a slowdown. A bit more realistic case (300 items with 10 mentions each, i.e. ye olde hellthread, chromium): ``` 300 items with links handling: Mount: 588.2700000074692ms, destroy: 3.9049999904818833ms, avg 1.9609000000248973ms - 0.013016666634939611ms per item 300 items without links handling: Mount: 80.83499997155741ms, destroy: 1.2250000145286322ms, avg 0.26944999990519136ms - 0.004083333381762107ms per item 300 items plain v-html: Mount: 66.73999998020008ms, destroy: 0.3849999629892409ms, avg 0.22246666660066694ms - 0.0012833332099641364ms per item ``` and same 10-mentions, but 20 items (same browser): ``` 20 items with links handling: Mount: 61.0300000407733ms, destroy: 0.5600000149570405ms, avg 3.051500002038665ms - 0.028000000747852027ms per item 20 items without links handling: Mount: 8.83500004420057ms, destroy: 0.2799999783746898ms, avg 0.44175000221002847ms - 0.013999998918734491ms per item 20 items plain v-html: Mount: 10.869999998249114ms, destroy: 0.26999996043741703ms, avg 0.5434999999124557ms - 0.013499998021870852ms per item ``` Note that this does not include all the other stuff that comes with `Status` component. I'm also not testing custom emoji rendering because we do really want play-on-hover gifs there. What's your take, @shpuld?
shpuld commented 2021-08-12 13:17:10 +00:00
Owner
Copy link

A real realistic worst case with virtual scrolling is 20 statuses created or destroyed at once, and it's not a very significant worst case. the significant case is individual statuses being rendered and destroyed which should happen in real time optimally (so that user does not even notice the scrolling during normal use, not reality on mobile unfortunately).

it's really hard to say based on the numbers how good/bad it is for the real usage. would recommend checking it in perf tools if the rich content mount slice is significant or not.

A real realistic worst case with virtual scrolling is 20 statuses created or destroyed at once, and it's not a very significant worst case. the significant case is individual statuses being rendered and destroyed which should happen in real time optimally (so that user does not even notice the scrolling during normal use, not reality on mobile unfortunately). it's really hard to say based on the numbers how good/bad it is for the real usage. would recommend checking it in perf tools if the rich content mount slice is significant or not.
hj commented 2021-08-12 13:52:18 +00:00
Author
Owner
Copy link

image

Seems to be very insignificant compared to rest of the status actually. Big bar = status rendering time (49ms), small bars (0.75ms, 2ms) - richcontent. First richcontent is for user title, second for status body

status in question:
image

![image](/attachments/d562060b-66be-4772-9e54-d622a97a7874) Seems to be very insignificant compared to rest of the status actually. Big bar = status rendering time (49ms), small bars (0.75ms, 2ms) - richcontent. First richcontent is for user title, second for status body status in question: ![image](/attachments/3618ff4f-5571-4929-9031-037324c60d7e)
image.png
4.9 KiB
image.png
38 KiB
shpuld commented 2021-08-12 18:32:08 +00:00
Owner
Copy link

yeah that seems all fine then, thanks for checking

yeah that seems all fine then, thanks for checking
hj commented 2021-08-12 19:51:28 +00:00
Author
Owner
Copy link

fancy styles gone, for now.

fancy styles gone, for now.
hj commented 2021-08-12 19:52:51 +00:00
Author
Owner
Copy link

removed and simplified.

also all mentions are now grouped together into MentionsLine regardless of placement. This combined with removal of mention-removal code simplified both code and tests for it.

removed and simplified. also all mentions are now grouped together into `MentionsLine` regardless of placement. This combined with removal of mention-removal code simplified both code and tests for it.
hj commented 2021-08-12 20:08:28 +00:00
Author
Owner
Copy link

Only thing remaining is to fix extra mentions line that i fixed a little too hard and fix that one comment.

Only thing remaining is to fix extra mentions line that i fixed a little too hard and fix that one comment.
hj commented 2021-08-12 20:16:15 +00:00
Author
Owner
Copy link

Oh and maybe find way to integrate mentionsline collapsing with long post detector, right now post is considered long if original HTML is long, it doesn't consider that MentionsLine clamps length of mentions.

Oh and maybe find way to integrate mentionsline collapsing with long post detector, right now post is considered long if original HTML is long, it doesn't consider that MentionsLine clamps length of mentions.
hj commented 2021-08-15 00:07:07 +00:00
Author
Owner
Copy link

ok, i think all issues are resolved now.

i feel like i should mention this that performance testing doesn't exactly reveal that these changes also introduce additional re-renders and possible server requests - all mentions have "original" mode where they display original content as is and "new" mode where they apply user highlight, full handle popover etc. New mode only engages when FE gets to know about the user in question.

There's another bit that this new style always displays itself as @username, which fixes annoyance to some with misskey's full @username@instance display. The only problem is that it doesn't affect the character counter. Which MIGHT throw tall-status-hider off a bit.

I'll be still dogfooding this for good measure, but otherwise it should be good to go

ok, i think all issues are resolved now. i feel like i should mention this that performance testing doesn't exactly reveal that these changes also introduce additional re-renders and possible server requests - all mentions have "original" mode where they display original content as is and "new" mode where they apply user highlight, full handle popover etc. New mode only engages when FE gets to know about the user in question. There's another bit that this new style always displays itself as `@username`, which fixes annoyance to some with misskey's full `@username@instance` display. The only problem is that it doesn't affect the character counter. Which MIGHT throw tall-status-hider off a bit. I'll be still dogfooding this for good measure, but otherwise it should be good to go
hj commented 2021-08-17 18:10:37 +00:00
Author
Owner
Copy link

the only bug that i found so far is this:
image

This happens because we trim the text after mentionsline because space is added via MentionsLine's margin, this due to problem that we trim all empty strings while grouping mentions, and in this case if we have say <Mention/> <Mention/> <Link/> the space between last mention and a link will be removed and two will stick. I'm not sure how to handle this since we can't peek ahead, can't retcon previous items and can't really insert new items either.

the only bug that i found so far is this: ![image](/attachments/297db4f8-40e9-4021-9445-fa6fddd73e1e) This happens because we trim the text after mentionsline because space is added via `MentionsLine`'s margin, this due to problem that we trim all empty strings while grouping mentions, and in this case if we have say `<Mention/> <Mention/> <Link/>` the space between last mention and a link will be removed and two will stick. I'm not sure how to handle this since we can't peek ahead, can't retcon previous items and can't really insert new items either.
image.png
2.3 KiB
hj commented 2021-08-18 17:56:16 +00:00
Author
Owner
Copy link

fixed, we now save last known spacing and prepend it by surrounding result in fake '' tag

fixed, we now save last known spacing and prepend it by surrounding result in fake `''` tag
hj commented 2021-08-23 18:59:13 +00:00
Author
Owner
Copy link

Fixed last bits of regressions/issues that i could find. I think this is ready to go in develop. @shpuld

Fixed last bits of regressions/issues that i could find. I think this is ready to go in develop. @shpuld

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
BE Request

   
Backlog

   
Better behavior

   
Blocked by deps

   
Bug

   
Code Review

   
Discussion

   
Doing

   
Feature Request / Enhancement

   
In the shining bright future maybe

   
It's complicated

   
MS Edge

   
Mememoon (or other niche browsers)

   
Missing API

   
Missing feature

   
Missing l10n/i18n

   
NL1

Instance Admin Dashboard

  
NL2

Improved Mobile Experience

  
NL4

Improved instance customization and QoL options

  
Need to verify on develop

   
RELEASE BLOCKER

   
Reassign or Close

   
Refactor

   
Regression

   
Safari

   
To Do

   
User story

   
Waiting on godot

   
accessibility

https://git.pleroma.social/pleroma/pleroma-fe/-/snippets/7746

  
better documentation

   
chore

   
confirmed

   
easy ticket

   
incident

Denotes a disruption to IT services and the associated issues require immediate attention

  
l10n update

   
mastoapi

   
mobile

   
needs design

   
needs-info

   
needs-review

   
stupid
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 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
pleroma/pleroma-fe!2659
No description provided.