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

Upgrade non-admin parts of UI to Bootstrap 5 #4437

Closed
13 tasks
yuvipanda opened this issue Apr 28, 2023 · 12 comments · Fixed by #4774
Closed
13 tasks

Upgrade non-admin parts of UI to Bootstrap 5 #4437

yuvipanda opened this issue Apr 28, 2023 · 12 comments · Fixed by #4774

Comments

@yuvipanda
Copy link
Contributor

yuvipanda commented Apr 28, 2023

Proposed change

Currently, we are on Bootstrap 3, which is almost 10 years old. We should upgrade to Bootstrap 5!

This issue is focused on bootstrap 5 for our non-admin panel, as the admin panel is using react and already on bootstrap 4.

Switch to webpack for CSS customization

We customize our CSS with bootstrap with custom less stuff. We should switch to using webpack instead, which would allow us to use https://getbootstrap.com/docs/5.0/getting-started/webpack/ for customization.

Files to be modified

Now that the admin panel is react, this is simpler. The following template files need to be modified to support bootstrap 5

  • 404.html
  • admin.html
  • error.html
  • home.html
  • login.html
  • logout.html
  • not_running.html
  • oauth.html
  • page.html
  • spawn.html
  • spawn_pending.html
  • stop_pending.html
  • token.html
@yuvipanda
Copy link
Contributor Author

this might also be a good time to get rid of less

@yuvipanda
Copy link
Contributor Author

yeah ok we definitely need to get rid of less, for the theme to work.

@echarles
Copy link
Contributor

echarles commented May 1, 2023

FYI we are implementing the Admin UI as a separated service as discussed on #3427

The work is happening in https://github.com/datalayer/jupyter-manager and we are moving away from bootstrap to primer.

Just posting this here for visibility, not trying in any way to open a discussion on any React toolkit choice.

@yuvipanda
Copy link
Contributor Author

@echarles +1, I think tha'ts a great idea!

I'll scope this to the non-admin parts of the UI (default login page, options form, control panel, etc)

@yuvipanda yuvipanda changed the title Upgrade to Bootstrap 5 Upgrade non-admin parts of UI to Bootstrap 5 May 1, 2023
@echarles
Copy link
Contributor

echarles commented May 2, 2023

@yuvipanda our goal is to keep it that project as a separated service and cover more than the pure JupyterHub aspects. Therefor, it is not probable that our work would land in the JupyterHub source repo.

@yuvipanda
Copy link
Contributor Author

@echarles aaah, ok. Then I'll just ignore it for this repo :) Thanks!

@minrk
Copy link
Member

minrk commented May 5, 2023

We absolutely should do this. I've put some time into it a few times, but ran out of energy for the less->scss conversion.

My latest wip: main...minrk:jupyterhub:bs5 if anyone wants to take that and try to finish it.

@yuvipanda
Copy link
Contributor Author

After doing a bunch of other frontend work recently (rendering jupyterhub-fancy-profiles, I now think we should just convert our less to plain CSS (with CSS variables if needed), and completely just get rid of the LESS / SCSS pre-compilation situation. I think that will help quite a bit. Perhaps that's a useful intermediate step.

@yuvipanda
Copy link
Contributor Author

ok, I looked through all our .less flles, and am now pretty convinced we can get rid of less completely. The amount of complexity the preprocessor adds is so much for what we get out of it. I think it made sense pre CSS variables, but not anymore. We might have to do something slightly hacky for setting primary colors, but the total UI surface we have is so little that I think it's fine. So I think the next step here is to just get rid of less while still keeping us on bootstrap 3, and then that makes future migration easier.

@krassowski
Copy link
Contributor

Not sure what did you use less for, but CSS nesting from sass is now built in in CSS proper too https://developer.chrome.com/articles/css-nesting/ which is supported by all major browsers but support is too fresh to push to production this year https://caniuse.com/css-nesting

@echarles
Copy link
Contributor

If I may, "The less CSS, the best" :)

@minrk
Copy link
Member

minrk commented Sep 28, 2023

I don't think we really use or need LESS features for our own styles, but it is where old bootstrap theme variables are set, since they didn't use css variables back in the long ago.

But removing our LESS to pure CSS, and leaving only what was needed for the bootstrap theming (I think just variables.less, perhaps) ought to be a start.

I'm skeptical that a 'slightly' hacky substitute for variables.less is doable, but I'd be happy to be wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants