Upgrade to Keycloakify 11

loculus-project / loculus

An open-source software package to power microbial genomic databases

https://loculus.org

GNU Affero General Public License v3.0

37 stars 2 forks source link

Upgrade to Keycloakify 11 #3270

Closed fhennig closed 4 days ago

fhennig commented 4 days ago

resolves #3264

preview URL:

Summary

The migration guide says to just start over with the starter template, so I'm gonna do that.

Screenshot

PR Checklist

[ ] All necessary documentation has been adapted.
[ ] The implemented feature is covered by an appropriate test.

fhennig commented 4 days ago

going on lunch break.

I've added the v11 template, added tailwind, ejected a few pages, now it's ready to be adjust to how we want to style it.

I've noticed that the old page did styling without tailwind, I think this is an improvement!

After lunch I'll get into remaking the style from the old design.

fhennig commented 4 days ago

Alright so for now I copied the colors.cjs file from the website - we need to find a solution for that. I think maybe we can refactor the colors into a dedicated module? I didn't want to do this here and now, I'll first look into the ORCID stuff now.

I fixed the background and button colors, so that already looks quite passable!

I just adjusted the PatternFly colors. If we wanted to not use PatternFly, I think we'd have to basically rewrite everything, so that doesn't seem sensible at the moment. Ideally we'd use DaisyUI like in the main UI, but not for now I'd say.

fhennig commented 4 days ago

Ahh, didn't add the Dockerfile yet, I'll get to that now

corneliusroemer commented 4 days ago

What I'd focus on first is get a keycloakify v11 setup that works as a theme in practice, with correct buttons and fields, then we can improve styling later. Styling is the last thing I'd do.

What's the reason you threw out yarn? We used it happily in v9: https://github.com/loculus-project/loculus/blob/main/keycloak/keycloakify/yarn.lock - I would stick as closely to what comes out of the box as possible to keep it maintainable. The fewer changes to the default starter the better!

fhennig commented 4 days ago

I am doing that now, I have already stopped changing visual stuff a couple of commits ago.

If we want something easily maintainable I'd suggest that we instead fork the starter and maintain our own change commits on top of theirs, always rebasing. we can then pull that fork in as a submodule.

I threw out yarn because we don't use it anywhere else and i basically just got rid of files, it didn't really add anything in complexity I thought, but of course it did change it a bit.

fhennig commented 4 days ago

Well, since I'm out of ideas for the CI I guess we could try going back to yarn and seeing if that works, but also I don't understand the underlying problem here.

I think I'll just stop working on it now, if you're in the mood to debug it and/or put yarn back in, feel free to do so!

theosanderson commented 4 days ago

https://github.com/loculus-project/loculus/pull/3271 fixes the tests

fhennig commented 4 days ago

closed in favor of https://github.com/loculus-project/loculus/pull/3272

corneliusroemer commented 3 days ago

Just out of curiosity, I managed to reproduce the ci errors in a local clone in docker Ubuntu arm64. So it's not a Github CI specific thing, I think that makes us a little less spooked @fhennig.

Ok, after a few more minutes of googling, checking that the versions are identical and config as well here's the reason: macOS file system is case insensitive. Ubuntu isn't. You had kcContext when KcContext was expected.

Now we can sleep safely again 😁