-
Notifications
You must be signed in to change notification settings - Fork 29
feat: Implement Authentication Component Suite for Storacha Console Toolkit #423
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
base: main
Are you sure you want to change the base?
feat: Implement Authentication Component Suite for Storacha Console Toolkit #423
Conversation
@Dhruv-Varshney-developer Could you please review the PR. |
307abdf
to
5aa8c7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly Looks good to me. Except that I'll prefer the code to refactored & simplified.
Few good practices:
- Big files (more than 200 lines of code) can be split into small components and those components can go in
/components
- Look for code where it can be deleted/simplified.
- If we can use Tailwind, libraries and toolkit and can reduce code, that's helpful.
5aa8c7d
to
6a5b61e
Compare
@Dhruv-Varshney-developer @travis Ready for the second round of review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great work, thank you!
Two things that need to be changed before I'm comfortable with this merging:
- Components that include CSS or styling belong in a separate package - this package intentionally does not include CSS and should not be expanded to include components with built-in styling choices. I'd recommend a new package called
react-styled
or something similar - Needs more usage examples - the
sign-up-in
updates are great! I'd like a bit more documentation and a few more examples of the functionality you're introducing here. In particular, a) this needs an example of how to swap in your own styles rather than using the prepackaged ones and b) this needs examples of and documentation for the "iframe" support
Happy to chat in higher bandwidth in Discord if that would be helpful!
import { useStorachaAuth } from '../components/StorachaAuth.js' | ||
|
||
/** | ||
* Enhanced authentication hook that provides additional functionality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what additional functionality does it provide?
Overview
Implements a complete authentication component suite (
StorachaAuth
) that transforms@storacha/ui
from a low-level SDK into a plug-and-play UI toolkit. This allows partner apps to embed Storacha console authentication features natively without double navigation UX friction.Features
useStorachaAuth
anduseStorachaAuthEnhanced
for custom implementationsImplementation
StorachaAuth
,StorachaAuthForm
,StorachaAuthSubmitted
,StorachaAuthEnsurer
useStorachaAuth
,useStorachaAuthEnhanced
with session trackingUsage
Testing
Screenshot
Part of #400
Closes #399