Rewrite BokehExample into transforms in the example app #8415
Conversation
Signed-off-by: Oskar Pawica <[email protected]>
| useSharedValue, | ||
| withRepeat, | ||
| withTiming, | ||
| type WithTimingConfig, |
There was a problem hiding this comment.
This probably needs to be moved to a separate import as we have an ESLint rule that forbids mixed imports.
| hue.value = withTiming(hue.value + randBetween(0, 100), config); | ||
| }; | ||
| const numberOfReps = shouldReduceMotion ? 1 : -1; | ||
| const config: WithTimingConfig = { duration, easing: Easing.linear }; |
There was a problem hiding this comment.
Do we really need to pass WithTimingConfig type here?
| return min + Math.random() * (max - min); | ||
| } | ||
|
|
||
| function getBokehCircleParams(): CircleProps { |
There was a problem hiding this comment.
Let's use make to convey that a new instance of something is created on each function call.
| function getBokehCircleParams(): CircleProps { | |
| function makeRandomCircleParams(): CircleProps { |
| left: 0, | ||
| top: 0, |
There was a problem hiding this comment.
Seems like this is not necessary
| left: 0, | |
| top: 0, |
| top.value = withTiming(top.value + randBetween(-100, 100), config); | ||
| hue.value = withTiming(hue.value + randBetween(0, 100), config); | ||
| }; | ||
| const numberOfReps = shouldReduceMotion ? 1 : -1; |
There was a problem hiding this comment.
I don't know why it was done this way but maybe let's rethink the expected behavior when the reduced motion setting is enabled?
| size: number; | ||
| opacity: number; |
There was a problem hiding this comment.
Might be worth moving these two to the top to separate the "static" props of circles from the ones that have "start" and "end" variants, just for the sake of readability.
2 participants
Summary
This PR optimizes the Bokeh example in the example app, by rewriting:
withRepeatinstead of JS intervalsCirclecomponent for potential reusability in some other examplestransformproperty instead ofleftandtopdirectlyTest plan