This repository has been archived by the owner on Dec 15, 2022. It is now read-only.
Share/join/leave portals via popover menu #99
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of showing one status bar icon per portal, always show exactly one status bar icon. This change paves the way for moving portal status/sharing/joining/leaving into a popover element accessed via the status bar icon.
jasonrudolph
commented
Oct 10, 2017
test/portal-binding-manager.test.js
Outdated
| const manager = new PortalBindingManager({client, workspace}) | ||
| const portalBindingPromise1 = manager.createHostPortalBinding() | ||
| const portalBindingPromise2 = manager.createHostPortalBinding() | ||
| assert.equal(portalBindingPromise1, portalBindingPromise2) |
There was a problem hiding this comment.
@as-cii: I added this test following our pairing session today. I don't know why it's failing. As far as I can tell, the two calls to manager.createHostPortalBinding() return the same Promise object, but this assertion is failing, so I must be overlooking something, . Any ideas what I might be doing wrong?
|
The mockups in #22 show the guest avatars overlapping each other slightly. I tried that out locally, and in practice, I think I like it better without the overlap: Overlapping guest avatars
Non-overlapping guest avatars
I'm gonna stick with the non-overlapping version for now. |
|
⚡️ |
added 5 commits
October 11, 2017 11:28
This will allow us to show a spinner as we get the host and guest portal bindings the first time the component is initialized.
added 4 commits
October 12, 2017 16:46
Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
Signed-off-by: Jason Rudolph <jasonrudolph@github.com>
We want to drive people toward the popover, while also ensuring that you can perform common operations via a command or keyboard shortcut. In the case of leaving a portal, the user can click the "leave" button in the popover *or* they can close the portal tab (e.g., "command+w"). Both actions will result in the user leaving the portal.
added 8 commits
October 13, 2017 11:04
With this commit we will now: * Stop showing a notification on `real-time:share-portal`. Instead, the popover will be revealed and we'll share the portal automatically on behalf of the user. Note that we will also stop automatically copying the portal id to the clipboard. * Stop showing a notification on `real-time:close-portal`. Instead, the popover will be revealed and we'll stop sharing automatically on behalf of the user. * Stop showing portal id prompt on `real-time:join-portal`. Instead, the popover will be revealed and we'll focus the portal ID text entry on behalf of the user.
# Conflicts: # package.json
If you trigger the command when you're already sharing a portal, we simply reveal the popover to remind the user that they already have a portal open.
Prior to this change, if you closed the portal via the command palette, we failed to hide the portal connection info.
jasonrudolph
changed the title
|
👏 👏 👏 Works great. I love it! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Big picture
Using a slimmed-down variant of the mockups in #22, this PR will allow users to perform the following tasks via a popover accessed via the portal icon in the status bar:
Low-level task list
This is a partial list of outstanding tasks. We'll continue to flesh out this list as we work through this PR.
client.onSignInChangeandclient.getLocalUserIdentityteletype-client#23, then depend on the newly published versionDeferred to future PRs
We want to also provide a legend that maps portal participants to their cursor colors. However, this PR is gonna be pretty big, so let's handle that enhancement in a follow-up PR.
Closes #18.