Jon/ui4/fix/getting-started #4745
Conversation
Jon-edge
force-pushed
the
jon/ui4/fix/getting-started
branch
from
6075630 to
cd42ef8
Compare
src/components/ui4/ButtonUi4.tsx
Outdated
| @@ -1,3 +1,7 @@ | |||
| /** | |||
| * IMPORTANT: Changes in this file MUST be synced with edge-react-gui! | |||
There was a problem hiding this comment.
We are already in edge-react-gui. Do you mean edge-login-ui-rn?
There was a problem hiding this comment.
I should just mention both to avoid copy pasta mistakes.
src/components/ui4/ButtonUi4.tsx
Outdated
| {!hideContent ? null : <ActivityIndicator color={spinnerColor} style={styles.spinnerCommon} />} | ||
| </LinearGradient> | ||
| </TouchableOpacity> | ||
| <MaybeEdgeAnim when={animProps != null} {...animProps}> |
There was a problem hiding this comment.
This layering makes no sense. The button user should just do <EdgeAnim><ButtonUi4/></EdgeAnim> themselves. There is no reason for the button to be animation-aware.
At the very least, the ButtonsViewUi4 should be in charge of adding the <EdgeAnim> wrapper.
| @@ -19,6 +20,8 @@ export interface ButtonInfo { | |||
| disabled?: boolean | |||
| spinner?: boolean | |||
| testID?: string | |||
|
|
|||
| animProps?: EdgeAnimProps | |||
There was a problem hiding this comment.
Optional: I don't especially like this, because it's too low level. We don't let the user tweak other button properties at this level of detail. Could it be something simpler, like an animate: boolean prop on the top-level ButtonsViewUi4 component? Then the buttons view is in charge of the visuals again, not the scene.
I don't think we should fix this now, because we are running out of time, but at least start thinking about other solutions.
There was a problem hiding this comment.
I agree, it would be nice to be simpler.
It's like this because each scene has its own distance timing depending on the number of items animating, I couldn't figure out how to get around that beyond giving animation handling to the SceneWrapper, and the SceneWrapper takes chunks of components like SectionView does.
This was the simplest in terms of integrating to other scenes that animated individual buttons without using a ButtonsViewUi4.
There was a problem hiding this comment.
Maybe instead we give ButtonsViewUi4 an animStartDistance: number?
There was a problem hiding this comment.
Yeah, something like this would be good.
| @@ -394,7 +404,7 @@ const SectionCoverAnimated = styled(Animated.View)<{ swipeOffset: SharedValue<nu | |||
| justifyContent: 'space-between', | |||
| backgroundColor: '#0F1D26', | |||
| paddingVertical: theme.rem(1), | |||
| paddingBottom: insets.bottom, | |||
| paddingBottom: insets.bottom + themeRem, | |||
There was a problem hiding this comment.
Please use theme.rem(1), like on the line above. We use the themeRem in animated props because we can't call methods across the reanimated bridge, so theme.rem() is unusable. Otherwise prefer the normal theme.rem(1) syntax.
Jon-edge
force-pushed
the
jon/ui4/fix/getting-started
branch
from
cd42ef8 to
d7ab5f9
Compare
| spinner?: boolean | ||
| testID?: string | ||
|
|
||
| animDistanceStart?: number | ||
| } | ||
|
|
||
| interface Props { | ||
| // Specifies whether the component should be positioned absolutely. | ||
| // Default value is false. | ||
| absolute?: boolean |
There was a problem hiding this comment.
Removed this fade prop because our animations effectively replace it, and it's
not used anyway.
Jon-edge
force-pushed
the
jon/ui4/fix/getting-started
branch
7 times, most recently
from
341c11b to
ca5fc07
Compare
Omitting the "sceneMargin"->"parentType" prop type change for now to avoid extra changes.
Support animation of individual buttons according to the specified timing from the caller scenes.
Don't include extra horizontal padding in the tertiary button so that the column layout looks like a better match with tertiary vs primary/secondary widths. Could benefit from a reorganization of the style heirarchy here but this is a low impact fix.
Jon-edge
force-pushed
the
jon/ui4/fix/getting-started
branch
from
ca5fc07 to
a7bd0b2
Compare
Looks really weird. It already looks like it's animating from all the gesture movements without EdgeAnim.
Jon-edge
force-pushed
the
jon/ui4/fix/getting-started
branch
from
a7bd0b2 to
09e4a58
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: