Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not condense single messages #4313

Merged
merged 1 commit into from
Sep 29, 2021
Merged

Conversation

supertassu
Copy link
Contributor

Skip condensing into groups of one message. It doesn't save much (if any) space but makes the mode changes harder to see without clicking.

Old:
Screenshot from 2021-09-18 18-08-58
New:
Screenshot from 2021-09-18 18-06-24

@MaxLeiter MaxLeiter added this to the 4.3.0 milestone Sep 18, 2021
@fnutt
Copy link
Contributor

fnutt commented Sep 21, 2021

Even though I like this PR, it's not a PR on the path of the optimal solution. A far better solution, short term, would be to actually group similar actions and just list them all. If more than one user, then add "and" before last user.

Example 1:

@User1-newnick,+User2, User5 and User12 has joined the channel
%User8 is now known as %User9-newnick and User6 is now known as User7
@user13 opped, +User20 voiced
User19 has quit

Example 2:

@User1-newnick and +User2 has joined the channel
User14, User15, User16, User17 and User18 has quit

The above examples are my suggestions based on a lenghty conversation over at #1685. They are far more simple and should require a lot less work to implement. On long term we can maybe look into far more condensed ways of showing actions.

@fnutt
Copy link
Contributor

fnutt commented Sep 21, 2021

From IRC #thelounge today:

10:45 fnutt: bookworm, MaxLeiter: Thoughts on #4313 (comment) ?
10:45 fnutt: Not sure if PR author is up for the task though.
10:46 fnutt: He seems to not be in this channel, sadly.
10:47 majavah: I am
10:47 fnutt: Lovely :) What are your thoughts on this, if I may ask?
10:49 fnutt: I'm not sure how much code it would require to implement it. However, it for sure is more along the path the former maintainers made it clear they wanted it (because the suggestions in the earlier PR went waaaay overboard).
10:52 majavah: That's a good idea for smaller channels, but I'm a bit worried on how it will work for large channels where there might be dozens of joins/quits/etc in one collapsed message - I still want to hide them to one line
10:53 fnutt: Would this be better?
10:54 fnutt: @User1-newnick,+User2, User5 and User12 has joined the channel | %User8 is now known as %User9-newnick and User6 is now known as User7 | @user13 opped, +User20 voiced | User19 has quit
10:54 fnutt: That way you have it one-line.
10:55 fnutt: Or you can... @User1-newnick,+User2, User5, User48, User12 and 10 others has joined the channel | etc... etc...
10:56 majavah: that might become a rather long line, but probably yeah, it's better
10:56 fnutt: If more than 5 users per action, add "and X others".
10:58 fnutt: I don't mind whatever solution we get in the end, but it would be better to work on solutions as above rather than hotfixing the one-user-one-action line, if my opinion.
10:58 fnutt: in my opinion*
10:58 majavah: fair :)
10:59 fnutt: I don't make decisions here though.
10:59 fnutt: Just out of curiosity, is this something you would have time and knowledge to implement?
11:00 majavah: I'm rather busy in the next week or two (exams) but after that, probably yeah

@termermc
Copy link

My preference is actually what the original PR contained. The reason is that most join and leave messages are basically just garbage that I don't want to see that makes it harder to follow. If I want to see who left and joined, I click to expand. if it's a single leave or join then it should be listed like this PR implements.

@MaxLeiter
Copy link
Member

I agree with @termermc, but will leave this open for a fewish days for discussion in case people feel strongly.

@MaxLeiter MaxLeiter added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Sep 21, 2021
@MaxLeiter
Copy link
Member

@supertassu do you have interest in implementing what @fnutt suggested, or should I review/merge this as-is?

@supertassu
Copy link
Contributor Author

@MaxLeiter I think this is fine to merge now. I might be interested in further improvements in the future, for now this still feels an improvement to the status quo.

@MaxLeiter MaxLeiter merged commit 7873847 into thelounge:master Sep 29, 2021
@MaxLeiter
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants