feat(react): add loading states, update SignIn and sample app, Add rollup for react. #15
Conversation
add skeletons for basic auth, totp.
…ing for SignIn component
…ntrol footer and logo
… when initializing
| enableConsoleBranding: authClientConfig?.enableConsoleBranding ?? true, | ||
| enableConsoleTextBranding: authClientConfig?.enableConsoleTextBranding ?? true, |
There was a problem hiding this comment.
Do we need the console part here? How about enableBranding and enableTextBranding? @brionmario @dasuni-30 WDYT?
There was a problem hiding this comment.
Branding is taken from props as well.
Need to specify this is coming from asgardeo/IS, right?
| external: ['react', 'react-dom'], | ||
| input: 'src/index.ts', | ||
| onwarn(warning, warn) { | ||
| // Suppress this error message... there are hundreds of them |
There was a problem hiding this comment.
Any reason to supress this error?
| } | ||
|
|
||
| setIsComponentLoading(true); | ||
| authContext.setIsAuthLoading(true); |
There was a problem hiding this comment.
Better to expose it directly from the context rather than using authContext.setIsAuthLoading
| @keyframes fade-in { | ||
| to { | ||
| opacity: 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Better not to add animations IMO as this could clash with the application's aesthetic. That should be on the hands of the app developer
| @keyframes fade-in { | ||
| to { | ||
| opacity: 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Same comment as above. Check in other places
|
|
||
| const [modalVisible, setModalVisible] = useState<boolean>(false); | ||
|
|
||
| const authContext: AuthContext | undefined = useContext(AsgardeoContext); |
There was a problem hiding this comment.
Instead of directly referring context, can't we use a custom hook to get these values? it looks more readable and elegant
| sessionStorage.clear(); | ||
| window.location.reload(); | ||
| if (contextValue.onSignOutRef.current) { | ||
| contextValue.onSignOutRef.current(); |
There was a problem hiding this comment.
What is the purpose of this?
3 participants
Purpose
Related Issues
Related PRs
Checklist
Security checks