search

jupyterhub / jupyter-remote-desktop-proxy

Run a Linux Desktop on a JupyterHub
BSD 3-Clause "New" or "Revised" License
116 stars 106 forks source link

Add Hub/Lab/Notebook navigation buttons to VNC top bar #7

Closed unode closed 9 months ago

unode commented 3 years ago

This PR adds navigation buttons to the VNC client top bar. Without it there are no links to navigate elsewhere or stop the container.

screenshot_2021-04-22_16-00-25_151009480

The CSS is a bit forced since buttons are using position: fixed with pixel level alignment.

CC'ing @titansmc

welcome[bot] commented 3 years ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

manics commented 3 years ago

Nice idea!

We should bear in mind this extension can be used outside JupyterHub, and also that lab or notebook may not be installed, so I think it's better not to build the full navigation menu in this extension, that should really be the role of the parent application.

How about if instead it's just a single Home button that goes to the user's starting location ../?

unode commented 3 years ago

../ redirects back to the VNC session so doesn't work as intended. ../../../hub/home sends you to the JupyterHub control panel.

I'm happy to remove the other buttons.

github-actions[bot] commented 3 years ago

Binder :point_left: Launch a binder notebook on this branch for commit ed43fcdda4f4cc3ad049c8a61da8523db7b644c1

manics commented 3 years ago

Could you use javascript to construct the URL to the parent folder and perform the redirect instead? For example

const pathParts = window.location.href.split('/');
window.location = pathParts.slice(0, pathParts.length - 2).join('/');
unode commented 3 years ago

Seems like the binder setup is a little different from the one we have in-house and we end up with different URLs. My initial PR doesn't work on binder but works locally. @manics suggestion works on binder but not here.

@manics can you elaborate why a relative URL wouldn't work and we need to use JavaScript for this one? JavaScript seems overkill but I'm probably missing a use-case.

yuvipanda commented 9 months ago

Quite a bit later, but this is superseeded by https://github.com/jupyterhub/jupyter-remote-desktop-proxy/pull/78 and https://github.com/jupyterhub/jupyter-remote-desktop-proxy/pull/79.

manics commented 9 months ago

Done in https://github.com/jupyterhub/jupyter-remote-desktop-proxy/pull/79!