server changes to enable new JB integration #7711
Conversation
roboquat
added
release-note-none
team: IDE
team: webapp
|
We need one more change to the dashboard before it is complete. |
Codecov Report
@@ Coverage Diff @@
## main #7711 +/- ##
==========================================
- Coverage 11.54% 10.28% -1.26%
==========================================
Files 20 18 -2
Lines 1169 1001 -168
==========================================
- Hits 135 103 -32
+ Misses 1031 897 -134
+ Partials 3 1 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
akosyakov
force-pushed
the
ak/jb_meta_changes
branch
3 times, most recently
from
dcabf3e to
ed54390
Compare
|
/hold because I want to make sure that @gtsiolis review UI design for the ready page with VS Code Desktop |
|
Looking at UX changes in this now! 👀 |
|
I set my default default to goland, and open a workspace, it jump to |
|
@iQQBot thanks for reviewing 🙏 it is a regression |
|
@iQQBot I pushed the fix, building now. |
There was a problem hiding this comment.
Great work, @akosyakov! 🔮
It took me while to wrap my head around some new concepts we've introduced with the gateway, etc. Posted also a comment with questions and suggestions in this relevant discussion (internal). Cc @loujaybee
I've left below some comments that are mostly out of the scope of these changes. Feel free to tackle these here if they make sense, create follow up issues if they seem important to track, or leave them here for future reference. 🏓
Regarding the UX of the new elements added in this page, I'm pasting below how this could look like with some small style changes. Let me know if a spec file with detailed spacing and color suggestions could help. Feedback is welcome! 🏀
| BEFORE | AFTER |
|---|---|
 |
 |
Approving to unblock merging but holding in case we need someone else to take a closer look at the code changes.
/hold
| @@ -292,6 +303,7 @@ export default class StartWorkspace extends React.Component<StartWorkspaceProps, | |||
| <a target="_parent" href={this.state.workspace?.contextURL}><p className="w-56 truncate hover:text-blue-600 dark:hover:text-blue-400" >{this.state.workspace?.contextURL}</p></a> | |||
There was a problem hiding this comment.
nitpick: This is out of the scope of the changes in this PR and we can tackle this in a follow up issue but the workspace ID now wraps after the changes in #7391. Using w-56 truncate classes from the context URL should suffice.
There was a problem hiding this comment.
praise: Thanks, @akosyakov! This has been bothering me for days. ✨
| defaultDesktopIDE: "code-desktop" | ||
| desktopIDEs: ["code-desktop"] | ||
| installationSteps: [ | ||
| "Click <b>Allow</b> in the dialog by your browser.", |
There was a problem hiding this comment.
praise: Do not know where to comment for this but thanks so much for triggering the dialog without the need to click the primary button here! ✨
| desktopIDEs: ["code-desktop"] | ||
| installationSteps: [ | ||
| "Click <b>Allow</b> in the dialog by your browser.", | ||
| "If you don't see the dialog, make sure you have <a target='_blank' class='gp-link' href='https://code.visualstudio.com/download'>VS Code</a> installed on your machine.", |
There was a problem hiding this comment.
nitpick: Couldn't find where to comment about this and it's probably out of the scope of the changes in this PR but it could be more clear and useful now to change the title here to the suggestion from #6270 (comment) or make the action more clear and separate it from the state.
There was a problem hiding this comment.
Ok change phase to Running and added Opening Workspace... as status description. I also replaced Opening IDE... with Opening Workspace.... I don't think everybody know what IDE stands for.
There was a problem hiding this comment.
I'd expect the Opening Workspace ... heading to be closer to the instructions and the action buttons, below the workspace entry as seen in the AFTER screenshot in #7711 (review), since this heading is more related to the actions than the actual lifecycle state of the workspace.
However, I think the current approach is also ok and not worth holding this back for now. We can always discuss this in depth in #6602. Cc @loujaybee
| PROPOSED | CURRENT |
|---|---|
 |
 |
|
LGTM label has been added. Git tree hash: 82197cf8c9960682266aa9d35a3cb492596c244c
|
akosyakov
force-pushed
the
ak/jb_meta_changes
branch
from
7fae301 to
f5f61e2
Compare
|
@gtsiolis I think I addressed all your feedback, could you have a look again please? |
akosyakov
force-pushed
the
ak/jb_meta_changes
branch
from
f5f61e2 to
ac97da9
Compare
|
UX looks great, @akosyakov! 🏁 Left one minor comment in #7711 (comment) but I don't think it's worth holding this PR back for that comment. |
|
Linked: #7708 looks like this will resolve that issue now, correct? |
|
/lgtm Changes to server look good. Just verified with how to test quickly. |
|
LGTM label has been added. Git tree hash: dda2728b4cb024fdc7dc4dd59483947621d49fb3
|
|
/unhold |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akosyakov, AlexTugarev, gtsiolis, iQQBot Associated issue: #7362 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
It is an extract from #7362 with changes which should be deployed by @gitpod-io/engineering-meta team before.
There are following changes:
getOwnerTokenmethod is added to allow Gitpod plugin to fetch it for SSH authreferrer:<client>(:<IDE>)?, right now onlyjetbrains-gatewayis supported, so one can start intellij directly by prefixing URL withgitpod.io#referrer:jetbrains-gateway:intellij/. Referrer prefix is also used to track clients access, so later can be reused for any derivative of VS Code for instance.Related Issue(s)
Related to #7362, fixes #7525
How to test
ak-jb-meta-changes.staging.gitpod-dev.com#referrer:jetbrains-gateway:intellij/to check that it actually starts with Intellij. In segment for staging untrusted you can then findide-referrerrecords.Release Notes
Documentation
/werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe