-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: onboarding screens in storybook #126
Conversation
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
@@ -45,9 +46,21 @@ const config = createTamagui({ | |||
fonts: { | |||
default: fontOpenSans, | |||
heading: fontRaleway, | |||
// Somehow adding body font gives build errors?! | |||
// body: fontOpenSans, |
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.
😭
<XStack display={state === 'enterPin' ? 'flex' : 'none'} gap="$3" justifyContent="center"> | ||
{pin.map((digit, index) => ( | ||
// biome-ignore lint/suspicious/noArrayIndexKey: index is the correct key here | ||
<YStack key={index} maxWidth={35} flex-1 justifyContent="center"> |
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.
IIRC @janrtvld this caused some very very weird errors in the idcrypt pin screen right?
</YStack> | ||
<YStack flex={9} mt="$5" onPress={focusInput}> | ||
<XStack justifyContent="center" gap="$2"> | ||
{new Array(pinLength).fill(0).map((_, i) => ( |
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.
Same here.
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.
You mean using key as index? It can cause glitches yes. But in this case we don't have a proper id. It would basically be the same as creating a custom array of objects where we add id: 0, id: 1. The pin is a stable array and thus it's OK in this case AFAIK
@@ -45,9 +46,21 @@ const config = createTamagui({ | |||
fonts: { | |||
default: fontOpenSans, | |||
heading: fontRaleway, | |||
// Somehow adding body font gives build errors?! |
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.
Have you already found the cause?
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.
No :/
apps/funke/tamagui.config.ts
Outdated
light: { | ||
...tokens.color, | ||
|
||
// Button | ||
buttonHeight: 53, | ||
|
||
// Button.Outline | ||
buttonOutlineBackgroundColor: hexColors['grey-100'], | ||
buttonOutlineBorderColor: '#ebedef', | ||
buttonOutlineTextColor: hexColors['grey-900'], | ||
}, |
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 button is this? Can't we do this in the button component itself?
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.
Also ideally we don't set fixed heights on buttons. I mostly used padding to resize it.
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.
Also ideally we don't set fixed heights on buttons. I mostly used padding to resize it.
Ah okay, took your design too literally then :)
What button is this? Can't we do this in the button component itself?
The outline button. We can't really do it as it changes based on the config and it's very different for Paradym and Funke.
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.
But if you have suggestions / concrete implementation proposal I'm all ears
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.
Ah this is the button with the icon? And i'm guessing the icon makes it higher than the regular button with only text.
For one-time overrides like this i'd pass in the height as a prop where you use it. It's often better than cluttering your config with values (imo).
The colouring is harder. Again you could use a prop override, but you don't want to do that all the time. As you also have the height override, it might be better to just make a new button variant that fits the requirements.
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.
And i'm guessing the icon makes it higher than the regular button with only text.
No not necessarily . I just set the button height for all buttons as it was very specific in hte design and the same height everywhere. Didn't know it was meant to be set with padding. So will update it.
For the outline button with icon it is still important to set the height/width i guess to make it squre ( as most icons are not square). I'll look into a new button variant
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.
Maybe you can look at this when you're back. I just can't get it to work nicely. If you could make it work with both Paradym and Funke styling without setting all the custom props that would be great. I now kept it at buttonHeight, and removed the other props (so the button outline is same as paradym button).
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
This adds the screens for onboarding flow. As a next steps I want to integrate them into the funke wallet, and also move them out of the stories.
So it's a bit of an in-between state, but I want to merge it in iterations.
See the video for the implemented flow
Screen.Recording.2024-07-23.at.13.51.19.mov