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

Cleanup the UI to be much nicer #78

Merged
merged 14 commits into from
Feb 5, 2024
Merged

Cleanup the UI to be much nicer #78

merged 14 commits into from
Feb 5, 2024

Conversation

yuvipanda
Copy link
Contributor

@yuvipanda yuvipanda commented Feb 3, 2024

Built on top of #77

image

This PR is a combination of JS cleanup as well as UI cleanup.

  • Remove ability to connect to arbitrary hosts and ports via query params. We already required websockify anyway, so this never worked. Plus, we only care about using the viewer for our own purposes, not as a general purpose novnc thing
  • Get rid of the ctrlAltDelete function. It's not particularly useful in this context. I will work on adding more UI here, but this location is a good place for other buttons
  • Add eslint for pre-commit and fix the one issue it found
  • Stop showing the 'desktop name' up top. This is the hostname, and not particularly useful.
  • Have a cleaner looking menu
  • Put the Jupyter Logo in there, clicking which takes you back to jupyterlab / notebook!
  • Use floating-ui to make the clipboard popover much nicer. This is the same library used by bootstrap (used to be popper.js). I actually spent some time trying to port the whole thing to bootstrap, but that caused more problems than it solved.

Copy link

github-actions bot commented Feb 3, 2024

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

I will automatically update this comment whenever this PR is modified

Binder 👈 Launch a binder notebook on this branch for commit 0faedee

Binder 👈 Launch a binder notebook on this branch for commit 0e938ce

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

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

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

Binder 👈 Launch a binder notebook on this branch for commit 0d9747e

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

Binder 👈 Launch a binder notebook on this branch for commit 651cb08

Binder 👈 Launch a binder notebook on this branch for commit 408a4fc

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

Binder 👈 Launch a binder notebook on this branch for commit 7c28872

Binder 👈 Launch a binder notebook on this branch for commit 4d1a878

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

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

@yuvipanda yuvipanda mentioned this pull request Feb 3, 2024
@yuvipanda yuvipanda requested a review from manics February 3, 2024 18:22
@yuvipanda yuvipanda marked this pull request as draft February 3, 2024 21:37
@yuvipanda yuvipanda changed the title Cleanup JS just a bit Cleanup the UI to be much nicer Feb 3, 2024
@yuvipanda yuvipanda marked this pull request as ready for review February 3, 2024 23:22
- Remove ability to connect to arbitrary hosts and ports
  via query params. We already required websockify anyway, so
  this never worked. Plus, we only care about using the viewer for
  our own purposes, not as a general purpose novnc thing
- Get rid of the ctrlAltDelete function. It's not particularly useful
  in this context. I will work on adding more UI here, but this
  location is a good place for other buttons
This is almost always a random sequence of strings and is
not useful.
Was missed when I undid all the queryparam stuff
- Include a CSS reset
- Use Jupyter Brand Colors
- Add a helpful bit of info for clipboard popout
- Uses floating-ui for a proper popover for the clipboard
- Sort out the menu to look much better
- Has a hover state and an active state
@yuvipanda
Copy link
Contributor Author

I wanna move js to src but that's likely to cause a bunch of knock on effects in #79 so I'll do that after.

Base automatically changed from no-vendor to main February 5, 2024 13:21
@manics
Copy link
Member

manics commented Feb 5, 2024

Looks nice!
I'm unsure about removing Ctrl-Alt-Del, I've had to use it a few times but can't remember why. It could be hidden in a menu though if you don't want it too visible? Or perhaps make the Jupyter logo a dropdown menu?

@yuvipanda
Copy link
Contributor Author

@manics when I tried it, it showed me the logout window. And if I tried to logout, it mostly just bricked my desktop session - I think it stopped dbus and xfce-session, but not anything else. That's one of the primary reasons I removed it, as our handling of how these processes are supervised (they just double fork and go on their merry way) doesn't help.

I'm going to add a dropdown menu with lesser used options - primarily intended for a checkbox for 'scale to fit'. But I can put a ctrl-alt-delete button there too. Are you ok with that coming in a later PR?

Copy link
Member

@manics manics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK! I've adding a tracking issue #83

@manics manics merged commit 374c02d into main Feb 5, 2024
6 checks passed
@manics manics deleted the no-ctrl branch February 5, 2024 20:51
@yuvipanda
Copy link
Contributor Author

Yay, thank you, @manics :)

@yuvipanda yuvipanda added the enhancement New feature or request label Feb 6, 2024
yuvipanda added a commit to 2i2c-org/nasa-qgis-image that referenced this pull request Mar 29, 2024
There's a lot of helpful UI changes to the desktop that
haven't been released yet (primarily jupyterhub/jupyter-remote-desktop-proxy#78).
In particular, the hub control panel is much easier to access,
and so is the clipboard.

Until a release is made (jupyterhub/jupyter-remote-desktop-proxy#87),
we can install it directly from source
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants