CE-1262 Set up authentication page #6
Conversation
…' into feature/CE-1262-set-up-authentication-page
# Conflicts: # src/pages/root/navbar/navbar.html
| <style> | ||
| #preload { | ||
| position: fixed; |
There was a problem hiding this comment.
There was a problem hiding this comment.
No, it doesn't work there
There was a problem hiding this comment.
You can try to change this one. You will see overlapping white screen.
There was a problem hiding this comment.
I suspect you shouldn't be using Windicss classes in index.html.
There was a problem hiding this comment.
I used it and it seem to be ok except class fixed
There was a problem hiding this comment.
@Nutto55 - Did you test is locally where the scripts load instantly or over a slow connection? I don't think you're testing the problem I'm expecting.
# Conflicts: # CHANGELOG.md # package-lock.json # package.json # src/models/Stream.ts # src/models/index.ts # src/pages/root/root.html # src/pages/species-richness/species-richness.ts # windi.config.ts
| document.getElementById('preload').style.visibility = 'visible' | ||
| document.getElementById('app').style.visibility = 'hidden' |
There was a problem hiding this comment.
I'm unsure about the sequence here...
- Isn't
#preloadalreadyvisible? - Shouldn't
#appalready behidden? (why not sethiddenonappdirectly in the HTML?) - Aren't the Windicss classes on
#preloadembedded in main.ts? At this point external styles & scripts are still loading
| export default { | ||
| domain: 'auth.rfcx.org', | ||
| clientId: 'LiojdvNUserGnCaLj8ckcxeGPHOKitOc', | ||
| audience: 'https://rfcx.eu.auth0.com/api/v2/' | ||
| } |
There was a problem hiding this comment.
I think this should be in .env or private.
| const Auth0Plugin = await Auth0.init({ | ||
| redirectUri: window.location.origin, | ||
| onRedirectCallback: (appState) => { | ||
| void router.push(appState?.targetUrl ?? window.location.pathname) |
There was a problem hiding this comment.
void router-push(...)
This is cool. I could never get this working! I think it's because I've be using arrow functions.
| export const Auth0: Auth0Plugin = { | ||
| init, | ||
| routeGuard | ||
| } |
There was a problem hiding this comment.
How confident are you that this file is correct?
I don't envy you having to write this!
There was a problem hiding this comment.
The start script is from official example which is in vue2 and got migrate to vue3 by him: https://github.com/alexeyzimarev/auth0-vue3-ts/tree/master/src. I have try to write it in another way e.g. class based or group them in a function but it is too complicated. Maybe you have a better idea to refactor auth0 config.
There was a problem hiding this comment.
Not without spending a long time on it. Maybe we should review this in the future. Hopefully Auth0 will publish an "official" Vue 3 solution.
| </div> | ||
| <div class="flex items-center mx-4"> | ||
| <button | ||
| v-if="!auth?.isAuthenticated" |
There was a problem hiding this comment.
Can you avoid auth being null/undefined?
There was a problem hiding this comment.
No, because initial value of Auth in vuex is undefined which is the same as value when user haven't authenticate yet.
There was a problem hiding this comment.
Is auth the Auth0 object? I don't think that should be in Vuex.
Or is it a user?
FYI - I think Vue doesn't like undefined for Reactive fields. You might need to use null. Or maybe that's only a Vue 2 issue.
| url: string | ||
| } | ||
|
|
||
| const CORE = 'https://staging-api.rfcx.org' |
There was a problem hiding this comment.
This should be in .env
There was a problem hiding this comment.
Yep, It should be another separate task for setting the env for staging, production, etc.
| const Auth0Plugin = await Auth0.init({ | ||
| redirectUri: window.location.origin, | ||
| onRedirectCallback: (appState) => { | ||
| void router.push(appState?.targetUrl ?? window.location.pathname) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Not keen on making the auth plugin here. This logic should be extracted. You could import createAuthPlugin so that you can still pass in the router, default URL, etc.
There was a problem hiding this comment.
If there is an outstanding issue, I would say it's this one: #6 (comment). I'm unfamiliar with Auth0 code (and I'm suspicious that maybe no one else has reviewed it either). Maybe we can discuss it after I start proper.
Other than that, I wrote a lot & learned quite a lot :) But all my comments are either questions or tips. I don't think there's anything blocking merge, so I will approve.
✅ DoD
📝 Summary
📸 Screenshots
🛑 Problems
💡 More ideas
vuexandpiniaand end up with vuex because I am more familiar with this tool + vuex is implement well to support vue 3 in my opinion.