Skip to content
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

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

Closed
wants to merge 1 commit into from

Conversation

unode
Copy link

@unode unode commented Apr 22, 2021

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
Copy link

welcome bot commented Apr 22, 2021

Thanks for submitting your first pull request! You are awesome! 🤗

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! 👋

Welcome to the Jupyter community! 🎉

@manics
Copy link
Member

manics commented Apr 23, 2021

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
Copy link
Author

unode commented Apr 23, 2021

../ 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
Copy link

Binder 👈 Launch a binder notebook on this branch for commit ed43fcd

@manics
Copy link
Member

manics commented May 2, 2021

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
Copy link
Author

unode commented May 3, 2021

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 added a commit that referenced this pull request Feb 3, 2024
We have been vendoring noVNC to get vnc_lite.html,
maintaining a patch file so we could upgrade the version of
noVNC used if needed.

noVNC publishes a JS library [on
npm](https://www.npmjs.com/package/@novnc/novnc)
that we can easily use instead, and stop vendoring the whole
package! This brings us the following massive advantages:

1. No more vendoring an entire package in!
2. Upgrades of noVNC client become practically trivial
3. We can change the frontend as we wish much more easily,
   compared to having to maintain a patch file as we do now.
   PRs like
   #7
   become easier.

This PR:

- Adds a `package.json` to include the noVNC js package
- Adds a `webpack.config.js` to provide JS bundling, so we can just ship
  a single `viewer.js` file that has everything.
- Adds appropriate bits (stolen and adapted from jupyterhub's config)
  to package the built JS in the final wheel as necessary.
- Upgrade from noVNC 1.2 to 1.4, but otherwise keep everything the
  same. I just copied our patched `vnc_lite.html` into `index.html`,
  and just split out the JS / CSS.
- prettier with pre-commit has done its thing on the JS, CSS and
  HTML. This is fine, as I am hoping we can make more changes to the
  UI after this.
- Modify the `Dockerfile` to setup the conda env first, and then
  separately install the python package with pip. This makes local
  development *much* faster.
- Add a `.gitignore` (which this repo was missing), with
  node_modules included as well as the dist/ directory with the built
  JS.
@yuvipanda yuvipanda mentioned this pull request Feb 3, 2024
@yuvipanda
Copy link
Contributor

Quite a bit later, but this is superseeded by #78 and #79.

@manics
Copy link
Member

manics commented Feb 5, 2024

Done in #79!

@manics manics closed this Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants