Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add status bar indicator when sharing or joining a portal #15

Merged
merged 8 commits into from Jul 11, 2017

Conversation

as-cii
Copy link
Contributor

@as-cii as-cii commented Jul 7, 2017

As per our roadmap, with this pull-request the real-time package will now show a little radio tower icon in the status-bar whenever a portal is active:

screen shot 2017-07-07 at 15 25 19

Hovering over the icon shows the following tooltip:

screen shot 2017-07-07 at 15 27 41

And, as the tooltip says, clicking on the icon will copy the portal id to the user's clipboard.

As you can notice, removeStatusBarIndicatorForPortal is only called on guest portals after the host disconnects. This is because we currently have no way of destroying a portal on the host machine; once we add support for it, however, it should be sufficient to call the above method as soon as users ask to stop sharing their portal.

@as-cii as-cii requested a review from jasonrudolph July 7, 2017 13:39
@nathansobo
Copy link
Contributor

Nice!

Copy link
Contributor

@jasonrudolph jasonrudolph left a comment

Choose a reason for hiding this comment

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

The code looks great! ⚡

Should we deal with the scenario where a guest is participating in multiple portals at once? In that scenario, it seems like the status bar icon would need to expose a different portal ID based on which of the guest portals is currently active (i.e., which guest portal is the active pane item). (Note: There's also the scenario where none of the guest portal pane items are the active pane item. 🤔)

Similarly, should we deal with the scenario where the user is a portal host and a portal guest at the same time?

Honestly, that last scenario is strange enough that we may want to explicitly disallow it (e.g., prevent a user from sharing a portal while they're currently a guest, and prevent a user from joining a portal while they're currently a host). If we do disallow it, it make sense to handle that in a separate PR.

What do you think?

@as-cii
Copy link
Contributor Author

as-cii commented Jul 10, 2017

Should we deal with the scenario where a guest is participating in multiple portals at once? In that scenario, it seems like the status bar icon would need to expose a different portal ID based on which of the guest portals is currently active (i.e., which guest portal is the active pane item). (Note: There's also the scenario where none of the guest portal pane items are the active pane item. 🤔)

Good point. 👍

Right now, the code shows multiple radio towers when the user joins many portals, independently of what is the active pane item. I can see why that could become confusing, though: seeing two icons at the same time without being able to know which tab belongs to which radio tower is somewhat weird.

At the same time, I feel like there is something nice about not tying the icon to the currently active pane item. For instance, if users switch to an item that does not belong to a portal (e.g. a local buffer) and we remove the icon, they may think the portal is gone. Viceversa, by keeping the icon around we make users aware of their connection to the portal, even when they are editing something else.

What do you think? After we implement authentication and presence, I was imagining that, in order to avoid confusion with multiple radio tower icons, we could have a different tooltip description for each of them that indicates what user the corresponding portal belongs to (e.g. I could have two icons and they would show respectively You are connected to Jason's portal and You are connected to Nathan's portal).

Antonio Scandurra added 3 commits July 10, 2017 15:19
Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
@nathansobo
Copy link
Contributor

I was actually imagining bi-directional portals being a pretty common usage pattern once we enable live branches, so that people could see what the other person is doing in one pane while navigating in another.

As for the UX, maybe eventually the tooltip could be a flyout menu similar to what we do with the GitHub package. It could list the portals you're connected to along with your own portal and who is connected to it.

Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
@as-cii
Copy link
Contributor Author

as-cii commented Jul 11, 2017

As for the UX, maybe eventually the tooltip could be a flyout menu similar to what we do with the GitHub package. It could list the portals you're connected to along with your own portal and who is connected to it.

I like this! 👍 In the meantime, we came up with the following UX:

kapture 2017-07-11 at 16 17 58

The idea is to show an icon for each portal, and to highlight the one that corresponds to the active tab. We (@jasonrudolph and I) think this is a good middle ground until we implement the flyout menu, so we'll go ahead and 🚢 this for now.

@as-cii as-cii merged commit 964ae79 into master Jul 11, 2017
@as-cii as-cii deleted the status-bar-indicator branch July 11, 2017 14:25
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.

None yet

3 participants