-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Wave 9] Onboarding Purpose Modal #76
Conversation
assets/images/simple-illustrations/simple-illustration__piggybank.svg
Outdated
Show resolved
Hide resolved
...(Array.isArray(wrapperStyle) ? wrapperStyle : [wrapperStyle]), | ||
!focused && (isHovered || pressed) && hoverAndPressStyle, |
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.
Hmm what is this for? Commenting it out but not seeing a difference
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.
The order of applying styles for MenuItem wrapper view was incorrect before, which resulted with you not being able to set both a wrapper style
and a specific hover / press style
that would override the style when certain action was met.
Setting a specific background color in the wrapper style disabled any changes for hovering over the component, due to it being added to styles AFTER the hover styling.
src/pages/OnboardingPurpose.tsx
Outdated
<View style={[styles.flex1, styles.dFlex, styles.flexGrow1, styles.mv5, shouldUseNarrowLayout ? styles.mh8 : styles.mh5]}> | ||
<View style={[styles.flex1]}> | ||
<View style={[shouldUseNarrowLayout ? styles.flexRow : styles.flexColumn, styles.mb5]}> | ||
<Text style={[styles.textHeroSmall]}>{translate('onboarding.purpose.title')} </Text> |
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.
As discussed, probably we should scroll the whole content (header + menu items) instead of just the menu items. Also, I'll double check on the sizing of the items, maybe they should fit without any scroll if the sizes are exactly like in figma
const theme = useTheme(); | ||
|
||
const handleGoBack = useCallback(() => { | ||
Navigation.goBack(); | ||
}, []); |
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.
Back navigation should always return to the first modal screen, and this doesn't work after page refresh. Let's maybe write explicitly to go back to name+surname screen, unless you have other ideas.
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.
I agree, it should fix any issues with reloading the page.
No description provided.