Further improve replies

[SimonBrandner]

Main issue which this fixes https://github.com/vector-im/element-web/issues/19074 Fixes https://github.com/vector-im/element-web/issues/21299 Fixes https://github.com/vector-im/element-web/issues/7159 Fixes https://github.com/vector-im/element-web/issues/18194 Fixes https://github.com/vector-im/element-web/issues/18027 Fixes https://github.com/vector-im/element-web/issues/19179

---

https://user-images.githubusercontent.com/25768714/137592147-190d4136-ded7-4ea7-9307-8a3831ebf445.mp4

---

Here's what your changelog entry will look like:

✨ Features

• Further improve replies (#6396). Fixes vector-im/element-web#19074, vector-im/element-web#21299, vector-im/element-web#7159, vector-im/element-web#18194 vector-im/element-web#18027 and vector-im/element-web#19179.

Preview: https://pr6396--matrix-react-sdk.netlify.app ⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

[robintown]

They're looking even better! :heart_eyes: Some thoughts:

The avatar is slightly off-center:

"In reply to themselves" overall sounds fine, but it feels weird when the message is coming from you. Maybe it should say "In reply to yourself" for that special case?

And while this does significantly improve the style of the "In reply to" bit to make it feel more clickable, I don't feel that it totally resolves https://github.com/vector-im/element-web/issues/7159, since the overall layout is still confusing to read. Perhaps it would be clearer if the "In reply to" text were just replaced with "Expand thread"?

[SimonBrandner]

And while this does significantly improve the style of the "In reply to" bit to make it feel more clickable, I don't feel that it totally resolves vector-im/element-web#7159, since the overall layout is still confusing to read. Perhaps it would be clearer if the "In reply to" text were just replaced with "Expand thread"?

Yeah, "Expand thread" sounds so much better!

[robintown]

For some reason avatar alignment is still broken for people without avatars:

[robintown]

~Also closes https://github.com/vector-im/element-web/issues/18194~ nevermind, they added additional concerns to the issue

[robintown]

The display names not being in bold looks a little strange to me IMO

[robintown]

Some more design comments:

The hover effect doesn't look great in light mode, and I wonder if there's something other than a brightness change that might look better in general. I feel like the hover effect, if any, should be some kind of background color change, but I get that that's tricky since there's already a hover effect on the overall event tile, sooo idk :woman_shrugging:

Also, the quoted messages appear to have sliiightly more padding on the bottom than on the top, even though they technically are symmetrical just going by the CSS, so that might benefit from some tuning

[SimonBrandner]

The hover effect doesn't look great in light mode, and I wonder if there's something other than a brightness change that might look better in general. I feel like the hover effect, if any, should be some kind of background color change, but I get that that's tricky since there's already a hover effect on the overall event tile, sooo idk

Yeah, it's not perfect, but I don't know how to solve it either. Let's if design has any ideas

Also, the quoted messages appear to have sliiightly more padding on the bottom than on the top, even though they technically are symmetrical just going by the CSS, so that might benefit from some tuning

Hopefully, that's better now

[robintown]

Padding looks good now, though the line height should be loosened up a little bit since descenders on certain letters get cut off now:

(something like line-height: $font-18px looks reasonable to me)

[robintown]

Also I had some inspiration for the hover effect: since the quoted message now has blockquote coloring, what if the effect of hovering over it was to remove that blockquote coloring, and return it back to the normal text color? I also tried out replacing the brightness(1.5) on the "Expand text" button with opacity(0.65). These hover effects are of course a bit more subtle than the current brightness change you have, but I personally think the subtlety works out really well:

Unhovered

"Expand thread" hovered

Quoted message hovered

[robintown]

I also wonder if replies to images could be improved while we're here, as they currently look a bit weird.

My initial thoughts are, since the filename and filesize labels were removed recently when the download button for images got moved, could we just get rid of the "unknown.png (x KB)" text entirely and unify this with the rest of the reply layouts?

[robintown]

Recently I've been thinking that the "Expand thread" text should just be removed, given that they add a lot of noise to the timeline, and because once actual threads are a thing, jumping through long reply chains is probably going to be a less common interaction. Not really sure :/

[Palid]

Maybe "Expand replies" instead of "Expand thread" will be good enough?

[SimonBrandner]

Maybe "Expand replies" instead of "Expand thread" will be good enough?

@robintown. how does that sound to you?

[robintown]

Maybe? It's a small enough thing though that I think we should just wait for design to provide a more conclusive opinion

[MadLittleMods]

What's stopping this from shipping? This seems like a nice iterative improvement and the expand/collapse stuff is being handled in https://github.com/vector-im/element-web/issues/18884 and https://github.com/matrix-org/matrix-react-sdk/pull/6701 while the text itself in this PR, if there is a more conclusive opinion, is an easy follow-up to change.

[robintown]

It's just waiting for design review currently

[MadLittleMods]

Added to the design inbox (doesn't show up here in the PR 🤷) -> https://github.com/matrix-org/design/projects/2 (other design inbox https://github.com/orgs/vector-im/projects/14)

[novocaine]

Hi @SimonBrandner

Whilst the video is great, for the purposes of us reviewing this PR it would be great to get a more formal text description of the problems you are aiming to solve and how you have solved them

Can you please create a new issue for this PR in the form we now propose for enhancements - listing each change you have made and why.

This will help the design team to more rapidly engage with what you're proposing, understand the context of your changes, and suggest any improvements or decline any changes.

Thanks!

[SimonBrandner]

@novocaine, I've made https://github.com/vector-im/element-web/issues/19074

[graue70]

I find the two lines on top of each other at the same level of indentation confusing. Why not use nested lines as in the following example?

Expand replies

Username Text I am replying to

My own reply

[robintown]

Also fixes https://github.com/vector-im/element-web/issues/19179

[codecov[bot]]

Codecov Report

Merging #6396 (6c16a41) into develop (3e31fdb) will decrease coverage by 0.06%. The diff coverage is 16.66%.

:exclamation: Current head 6c16a41 differs from pull request most recent head 8875f0b. Consider uploading reports for the commit 8875f0b to get more accurate results

@@             Coverage Diff             @@
##           develop    #6396      +/-   ##
===========================================
- Coverage    30.85%   30.78%   -0.07%     
===========================================
  Files          893      893              
  Lines        50793    50761      -32     
  Branches     12928    12917      -11     
===========================================
- Hits         15671    15627      -44     
- Misses       35122    35134      +12     

Impacted Files	Coverage Δ
src/components/views/messages/MImageReplyBody.tsx	9.09% <ø> (+2.84%)	:arrow_up:
src/components/views/rooms/ReplyTile.tsx	2.04% <0.00%> (+0.11%)	:arrow_up:
src/components/views/elements/ReplyChain.tsx	29.11% <50.00%> (+0.19%)	:arrow_up:
src/components/views/rooms/SearchBar.tsx	0.00% <0.00%> (-89.29%)	:arrow_down:
src/components/views/rooms/ReadReceiptGroup.tsx	8.33% <0.00%> (-21.67%)	:arrow_down:
src/contexts/RoomContext.ts	100.00% <0.00%> (ø)
src/components/structures/RoomView.tsx	29.65% <0.00%> (ø)
src/components/views/right_panel/UserInfo.tsx	29.23% <0.00%> (ø)
src/Lifecycle.ts	1.42% <0.00%> (+<0.01%)	:arrow_up:
...gs/handlers/AbstractLocalStorageSettingsHandler.ts	76.31% <0.00%> (+1.31%)	:arrow_up:

[robintown]

@janogarcia Here's what it looks like now:

Modern layout	Bubble layout

[robintown]

@janogarcia Done, please take another look when you get the chance