[BA-2338] Sign in with Base button Preact + React#5
Conversation
fan-zhang-sv
force-pushed
the
felix/buttons
branch
from
4301318 to
5360792
Compare
fan-zhang-sv
changed the title
🟡 Heimdall Review Status
|
fan-zhang-sv
requested review from
arjun-dureja,
cb-jake,
lukasrosario and
stephancill
and removed request for
lukasrosario
fan-zhang-sv
force-pushed
the
felix/buttons
branch
from
5360792 to
0f26494
Compare
| "typescript": "^5.1.6", | ||
| "vitest": "^2.1.9" | ||
| }, | ||
| "peerDependencies": { |
There was a problem hiding this comment.
is the plan to support other frameworks? if so we should either name this package based on react or use exports to export the react components vs vue, etc..
There was a problem hiding this comment.
ya, the plan is to create wrapper for vue, svelte, etc, just this PR only focus on preact and react wrapper
There was a problem hiding this comment.
we should use package.json>exports for each package then
| // biome-ignore lint/correctness/noUnusedImports: preact | ||
| import { h } from 'preact'; | ||
|
|
||
| export const TheSquare = ({ darkMode }: { darkMode: boolean }) => ( |
There was a problem hiding this comment.
If this is the base logo we should call it that vs something that is esoteric
| const LIGHT_MODE_BOARDER = '#1E2025'; | ||
| const DARK_MODE_BOARDER = '#282B31'; | ||
| const WHITE = '#FFF'; | ||
| const BLACK = '#000'; |
There was a problem hiding this comment.
maybe move this to a constants file that could be used by other ui libraries
| @@ -0,0 +1,14 @@ | |||
| // biome-ignore lint/correctness/noUnusedImports: preact | |||
| import { h } from 'preact'; | |||
There was a problem hiding this comment.
this is being consumed by preact implementation
| export type { SignInWithBaseButtonProps } from './types.js'; | ||
|
|
||
| // Preact components | ||
| export { SignInWithBaseButton as PreactSignInWithBaseButton } from './components/preact/SignInWithBaseButton.js'; |
There was a problem hiding this comment.
we should use separate export paths. i.e https://nodejs.org/api/packages.html#exports
| export type SignInWithBaseButtonProps = { | ||
| centered?: boolean; | ||
| transparent?: boolean; | ||
| darkMode?: boolean; |
There was a problem hiding this comment.
I think colorScheme is a bit more scalable long term.
| "web3" | ||
| ], | ||
| "type": "module", | ||
| "exports": { |
cb-jake
left a comment
cb-jake
left a comment
There was a problem hiding this comment.
looks good. Just a nit on directory structure. What are your thoughts
|
created a new one here #7 |
3 participants
Summary
How did you test your changes?