-
Notifications
You must be signed in to change notification settings - Fork 666
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
feat: add username modes #4835
base: master
Are you sure you want to change the base?
feat: add username modes #4835
Conversation
This adds username modes on both the chat message and the user list for to allow css styling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. I do have some comments, see below
Kindly let me know if there are any further changes I have to make to make this happen 😄 |
rebase the PR and fold the fixup commit into the main one. But no, I'm simply lacking time due to life. |
client/components/Username.vue
Outdated
@@ -1,6 +1,7 @@ | |||
<template> | |||
<span | |||
:class="['user', {[nickColor]: store.state.settings.coloredNicks}, {active: active}]" | |||
:class="['user ', {[nickColor]: store.state.settings.coloredNicks}, {active: active}]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you add a space here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a goof-up on my side, apparently I didn't remove it when I moved the user modes to data-mode attribute. Will fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one thing which is a bit funny now is that if a "mode" user speaks, they get the mode data-attribute set as expected.
However, if that user gets mentioned it does not.
Is this desired?
client/components/Username.vue
Outdated
@@ -72,7 +74,10 @@ export default defineComponent({ | |||
|
|||
const store = useStore(); | |||
|
|||
const modeName = constants.modeCharToName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modeName seems a poor name, why not keep it as modeCharToName?
It's a mapping, not a mode name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, I'll have it changed
"@": "op", | ||
"%": "half-op", | ||
"+": "voice", | ||
"": "normal", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are dropping normal from the list, did you check that this is safe to do and it is in fact never hit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake - normal was used on the css as a grouping name so I placed it back.
https://github.com/snowfudge/thelounge/blob/c426de9e7e18ad0393b1356575e2b0701321158f/client/css/style.css#L1809-L1811
I've also added back the "!" based on PR#354 because I don't want to break things.
Changed operator -> op as well because again, it's used on the default styling as a grouping name
- Also added back "!" based on PR thelounge#354 for the "odd network" that uses it (by omnicons)
This is intended, yes. Initial reason was to style the username when they say something in the server so it's easier to, for example, know when an admin or an op says something. If they ping us, we get notified anyway by default. If they ping let say voiced users, said users will get the notification, which is why I deem it unnecessary to add the data atribute user mode to any pinged users of the message. |
This adds username modes on both the chat message and the user list for to allow css styling. Users can then style names based on the username modes which only worked on the user list prior to this commit.
Currently it only exist on the userlist, e.g:
But not the chat message, e.g.:
This PR aims to add user mode status on the username element so it'll show something like this both on the user list and chat message list: