[RNMobile] Circle Progress for MediaUploadProgress #27429
Conversation
jd-alexander
added
the
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.19 MB ℹ️ View Unchanged
|
|
I see that the snapshots would be outdated with these changes. If we proceed, those would be updated. |
|
|
||
| const AnimatedCircle = Animated.createAnimatedComponent( Circle ); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
I didn't want to hardcode the stroke colors. At first, I tried putting them in a style sheet but it didn't work. Just to verify, I can add a stroke attribute to a style with say $blue-wordpress and that should work, right? I am going to re-add the stylesheet tomorrow so the stroke and fill values are stored there.
There was a problem hiding this comment.
Since stroke is a property on its own, you would need to assign it directly. Something like this:
stroke={ styles.circleProgress.stroke }
This I think should work
The same can be done with fill.
| import Svg, { Circle } from 'react-native-svg'; | ||
| import Animated from 'react-native-reanimated'; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Will remove this.
There was a problem hiding this comment.
Pretty nice spike @jd-alexander !
I have left a code comment about the strategy used to add the component to the tree. Other than that looks quite good.
- The RichText component wraps on Android. Similar issue to the one that occurred with the placeholder issue.
- If you type a really long title the progress circle goes off-screen so we would need to have some form of flexbox styling that stops the
RichTextcomponent from growing to the edge of the screen.
If we go down this path, I'd suggest we draw the ProgressCircle below the file name. This would avoid the issues mentioned before.
We also had the same issues on iOS that we had with the Download button, and the patch for that is pretty messy. (cc @iamthomasbishop )
Another small detail:
The circle feels like it's filling up in the opposite direction. Normally I think it would start at the top and fill clockwise 🤔
And, if we go down this path, we have also to consider right and centred alignments
Note that:
While this works, we are moving further away from the web way of doing uploads.
If we do an upload flow improvement mini-project, ideally we would create a compatible MediaUploadFlow component on mobile, that won't be able to present itself loading/progress indicators.
| const inlineProgressComponent = ( inlineView ) => { | ||
| return ( | ||
| <View style={ styles.inlineProgressContainer }> | ||
| { inlineView } | ||
| { isUploadInProgress && ( | ||
| <ProgressCircle progress={ progress } /> | ||
| ) } | ||
| </View> | ||
| ); | ||
| }; | ||
|
|
There was a problem hiding this comment.
I think it would be more flexible to go the other way around:
Instead of forcing the ProgressCircle component to be inline in a row with other components, we could give back the ProgressCircle component to the parent so it can place it anywhere it wants to, in the same way we do with the media source picker.
So instead of a new isInline prop, we can create a progressStyle property that defaults to top-bar, and also has circular and maybe none that could be used by the Audio block
| const { interpolate, multiply } = Animated; | ||
| const { PI } = Math; | ||
| const circumference = r * 2 * PI; | ||
| const α = interpolate( progress, { |
There was a problem hiding this comment.
Hah, interesting constant name :)
Maybe we should continue using latin letter 🤔
|
Thanks for the review here @etoledom 🙇♂️
With this said we can close this PR for now especially since we are sticking with the current behavior of the progress component for all blocks and we would be better served with n upload flow improvement mini-project as you said. If necessary logic and thoughts shared here can be revisited. |
WP Android wordpress-mobile/WordPress-Android#13516
Description
This PR introduces a circle progress component mode for the
MediaUploadProgresscomponent. This behavior works byHow has this been tested?
Screenshots
Noticeable issues
RichTextcomponent from growing to the edge of the screen.Checklist: