-
Notifications
You must be signed in to change notification settings - Fork 30
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(ws): Add "Connect" column to workspace table and display popup with workspace endpoints. #161
feat(ws): Add "Connect" column to workspace table and display popup with workspace endpoints. #161
Conversation
8e967e5
to
0ce39fd
Compare
0ce39fd
to
68357a9
Compare
68357a9
to
ba325f5
Compare
ba325f5
to
eea81bb
Compare
Following a discussion with @jenny-s51, we've decided to move the endpoints into the expanded table row. |
@Yael-F The intention of this button is to connect to the notebook instance (open a new tab), making it the single most important button on the page, it must be in the main row. The design I was thinking was a button with dropdown to select which port to connect to. NOTE: replace "Primary" with "Jupyter" (or whatever the first port is called), and the other actions being the other ports (or greyed out if there is only one). NOTE 2: we might also want some kind of "connect" icon on the left side of the button, but I am not sure if that will just clutter. |
@Yael-F I agree with @thesuperzapper! That is the primary action when I open the workspaces listing. |
09574e0
to
a933edc
Compare
Hey @thesuperzapper Jenn (the UX) suggested to label the dropdown with "Connect" (instead of the first endpoint) for better clarity, what do you say? |
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.
Hi @Yael-F , thanks for the PR!
Please check the npm run test
locally, it has a few errors. You can use npm run test:fix
to fix most of them. Also please rebase your branch with the latest commits.
workspace: Workspace; | ||
}; | ||
|
||
export const EndpointsDropdown: React.FunctionComponent<EndpointsDropdownProps> = ({workspace}) => { |
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.
Please consider renaming the file and component (and other associated types, like the EndpointsDropdownProps
) to WorkspaceConnectAction
.
a36531d
to
19f4e9a
Compare
…ace endpoints Signed-off-by: Yael <[email protected]>
…efault (main) endpoint Signed-off-by: Yael <[email protected]>
19f4e9a
to
79a5f0c
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ederign, paulovmr 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 |
/lgtm |
Fixes #150
This PR adds a "Connect" column to the workspace table, positioned before the Actions column.
The new column displays a list of endpoints on hover.
I've added a screenshot of the implementation. Please let me know if any further changes are needed.
