Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

IRC ui layout #4531

Merged
merged 31 commits into from
May 19, 2020
Merged

Conversation

JorikSchellekens
Copy link
Contributor

@JorikSchellekens JorikSchellekens commented Apr 30, 2020

This implements a nice irc style ui. Depends on both #4424 and element-hq/element-web#13350

image

Fixes element-hq/element-web#13552
Requires element-hq/element-web#13350

Live preview: https://riots.im/adhoc/irc-ui-3/

@JorikSchellekens
Copy link
Contributor Author

Still needs some fixes around replies and edits but it's mostly there

@t3chguy
Copy link
Member

t3chguy commented Apr 30, 2020

This is prettier than I imagined :D

This is why we shouldn't rely on regex
@JorikSchellekens
Copy link
Contributor Author

Addresses element-hq/element-web#13552

@JorikSchellekens JorikSchellekens requested a review from a team May 7, 2020 13:35
@JorikSchellekens JorikSchellekens self-assigned this May 7, 2020
@JorikSchellekens
Copy link
Contributor Author

There's now a way to disambiguate names by hovering over them.

@JorikSchellekens
Copy link
Contributor Author

Replies and edits fixed

@joepie91
Copy link

joepie91 commented May 7, 2020

Looks quite nice, though I'd suggest making the replies less bulky. This is how I addressed that problem in Neo, for example (which seemed to get positive responses from people):

Neo compact mode screenshot

@JorikSchellekens JorikSchellekens changed the title New IRC ui layout IRC ui layout May 8, 2020
@JorikSchellekens JorikSchellekens requested a review from a team May 13, 2020 13:42
@bwindels bwindels requested review from bwindels and removed request for a team May 15, 2020 08:39
@bwindels
Copy link
Contributor

Thanks for looking into this and refactoring the CSS in an area that hasn't received too much love in a while!

I am reviewing it now, but doing a quick visual scan between nightly and the adhoc build with IRC layout turned off (both with compact timeline mode on and off) reveals that the layout/spacing has changed a bit, especially in compact mode (if you flip between them in an image viewer it should be apparent). I appreciate it might be hard to keep it exactly the same, but the 2 things I would perhaps consider addressing would be:

  • The pill seems to have gained some extra padding on the bottom side.
  • The vertical spacing in compact mode seems to have increased noticeably.

The screenshots of what I am seeing:

Adhoc build, compact mode

adhoc-compact

Nightly build, compact mode

nightly-compact

Adhoc build, non-compact mode

adhoc-noncompact

Nightly build, non-compact mode

nightly-noncompact

Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for looking into this! The resize handle on the sender profile column is really cool!

Some nits and suggestions you could consider (in a follow-up PR would be OK) but code overall looks good!

One thing I would like to see addressed before this is merged (or really short after if we need this today) is the regression of the vertical spacing in the compact mode when irc layout is off as mentioned.

res/css/views/rooms/_GroupLayout.scss Show resolved Hide resolved
res/css/views/rooms/_GroupLayout.scss Outdated Show resolved Hide resolved
res/css/views/rooms/_IRCLayout.scss Outdated Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
src/components/views/rooms/EventTile.js Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
src/components/views/elements/ErrorBoundary.js Outdated Show resolved Hide resolved
res/css/views/rooms/_IRCLayout.scss Show resolved Hide resolved
- removed superfluous position and classes
- fixed compact view
- fixed event list summary avatar and text overlap
- fixed a problem where the mention list refuses to load.
@JorikSchellekens
Copy link
Contributor Author

Ah, the builds are failing because of the dispatcher move.

@bwindels
Copy link
Contributor

@JorikSchellekens did you manage to fix the vertical spacing issue mentioned above?

@bwindels
Copy link
Contributor

Ah, the builds are failing because of the dispatcher move.

Perhaps a rebase would help?

@JorikSchellekens
Copy link
Contributor Author

@JorikSchellekens did you manage to fix the vertical spacing issue mentioned above?

Yup that's fixed

@JorikSchellekens
Copy link
Contributor Author

I noticed the notification bar was completely messed up because of the irc layout. Spent most of the morning trying to fix it. It didn't work well so I've just disabled irc for anything which isn't a traditional room.

@bwindels bwindels self-requested a review May 19, 2020 16:24
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go assuming build succeeds.

@JorikSchellekens JorikSchellekens merged commit 5a1bf03 into matrix-org:develop May 19, 2020
turt2live added a commit that referenced this pull request May 21, 2020
Fixes element-hq/element-web#13736

This also fixes an unreported but complained about issue regarding the 'always show timestamps' option not working.

Looks like this regressed in #4531 when things got shuffled around.
@bitrage
Copy link

bitrage commented May 30, 2020

Looks great so far!

A simple observation using the nightly: the adjustable width of the nick-column is not persistent between room changes or client restarts.

Also this would profit immensely from a monospaced font option.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IRC style layout for message list
5 participants