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

Add MONITOR support and status icons for queries #4550

Open
wants to merge 88 commits into
base: master
Choose a base branch
from
Open

Conversation

MaxLeiter
Copy link
Member

@MaxLeiter MaxLeiter commented Apr 29, 2022

image

When a query is open the icon disappears from the sidebar and is moved to the titlebar:
image

There's also tooltips:
image

Away support:
image

Closes #337, closes #872 (#337 can't really be fully addressed -- tracking nick changes isn't really possible atm)

I also added requesting the draft/extended-monitor cap, so away messages will be sent to users in the monitor list. https://ircv3.net/specs/extensions/extended-monitor

Updated to typescript

@MaxLeiter MaxLeiter added the Status: needs-review PR not yet reviewed by enough maintainers label Apr 29, 2022
@brunnre8
Copy link
Member

I would prefer not to move the online icon away when you open a query. It's really helpful to scan a list of online people quickly (least I used that a lot for work in other chat like things) and you really want that all in the same place.

Duplicate the bubble, that's fine, but please don't move it.

@MaxLeiter
Copy link
Member Author

MaxLeiter commented Apr 29, 2022

It has to move anyways. Should it move to the left of the close icon?

image

@brunnre8
Copy link
Member

That's not what I meant by move, it's still in the same global position, namely the chan list. Yes it physically moves slightly to the left, but it still hugs the channel list entry.

What I meant by moving was your "let's portal it to some completely different UI element" and suddenly it's in the titlebar which has no relationship to the chanlist at all per se.

@brunnre8
Copy link
Member

On mobile specifically, when you have the chanlist open you don't see the titlebar of the open chan/query anymore. It gets dimmed and is under the chanlist

@MaxLeiter
Copy link
Member Author

MaxLeiter commented Apr 29, 2022

image

I've pushed away support in the sidebar. This uses a `who` call to query all users in a channel, as recommended by https://ircv3.net/specs/extensions/away-notify.html#description:
To fully track the away state of users, clients should:

1) Enable the away-notify capability at negotiation time.

2) Execute WHO when joining a channel to capture the current away state of all users in that channel.

3) Update state appropriately upon receiving an AWAY message

@dgw
Copy link
Contributor

dgw commented Apr 29, 2022

This is just a design thought: The badge would be more "scannable" if it lived either to the left of each nick or pinned to the far right edge like the unread-count indicators in the channel list.

Other than that nitpick, I'm fairly excited to see this new feature land.

@MaxLeiter
Copy link
Member Author

Fixed and updated screenshot in comment, @dgw

@dgw
Copy link
Contributor

dgw commented Apr 29, 2022

Beautiful, @MaxLeiter!

@brunnre8
Copy link
Member

indeed much better on the lefthand side in the user list

@aab12345
Copy link
Contributor

awesome work!!!

</template>
</div>
</div>
</aside>
</template>

<style>
.userlist {
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't change any of the CSS, it's copied from style.css

}

/* Status icon */
#chat .names .status {
Copy link
Member Author

Choose a reason for hiding this comment

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

this is new

@@ -23,5 +25,10 @@ export default {
network: Object,
message: Object,
},
computed: {
awayMessage() {
return this.message.text.trim();
Copy link
Member Author

Choose a reason for hiding this comment

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

this is not really necessary to shove in this PR, but we shouldn't show empty away messages, as it looks like ( )

@MaxLeiter MaxLeiter added this to To do in IRCv3 via automation Apr 30, 2022
@xPaw
Copy link
Member

xPaw commented Apr 30, 2022

I'm just gonna mention (non blocking), that different states should probably use different shapes/icons in the future, because colors alone are not good for accessibility or quick scanning.

Also, what about combining the status icons into the channel icon in the channel list, similar to how notification dot looks on the hamburger button.

@MaxLeiter
Copy link
Member Author

When this is merged I'll open a discussion on how it should look

@brettgilio
Copy link

Is there a reasonable way to revive the work in this branch?

@MaxLeiter
Copy link
Member Author

@brettgilio yeah, but its probably easiest to start fresh and just copy paste code over. Its on my eternal todo list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: needs-review PR not yet reviewed by enough maintainers
Projects
IRCv3
  
To do
7 participants