refactor: iOS Surface major refactoring
#3738
Conversation
|
Hey @DimitarNestorov, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The mobile version of example app from this branch is ready! You can see it here. |
DimitarNestorov
force-pushed
the
chore/button-width
branch
from
2b60477 to
8a8d924
Compare
DimitarNestorov
changed the title
Surface outer layerSurface styles
DimitarNestorov
force-pushed
the
chore/button-width
branch
2 times, most recently
from
e3c4a2a to
485a239
Compare
DimitarNestorov
changed the title
Surface stylesSurface major refactoring
DimitarNestorov
changed the title
Surface major refactoringSurface major refactoring
This comment was marked as resolved.
This comment was marked as resolved.
| return ( | ||
| <Animated.View | ||
| ref={ref} | ||
| style={outerLayerViewStyles} |
There was a problem hiding this comment.
Most likely, to get rid of the warning RCTView has a shadow set but cannot calculate shadow efficiently. I think the outer layer will need to have a backgroundColor style as well. However, it can require passing there also a borderRadius to sync it with the inner layer.
There was a problem hiding this comment.
This seems to work but AnimatedFAB depends on backgroundColor to be transparent. I say we still do it and open a separate issue to figure something out for AnimatedFAB.
There was a problem hiding this comment.
This seems to work but AnimatedFAB depends on backgroundColor to be transparent. I say we still do it and open a separate issue to figure something out for AnimatedFAB.
How to reproduce it on your branch?
There was a problem hiding this comment.
How to reproduce it on your branch?
I tried upgrading Expo but I didn't get warnings. Ended up creating an empty RN project and publishing Paper to npm: https://www.npmjs.com/package/@dimitarnestorov/react-native-paper/v/5.4.1-beta.0
Since transparent is hardcoded you need to go to node_modules/@dimitarnestorov/react-native-paper/src/components/FAB/AnimatedFAB.tsx and manually change the colour.
App.tsx:
import React from 'react';
import {
AnimatedFAB,
} from '@dimitarnestorov/react-native-paper';
function App(): JSX.Element {
return (
<AnimatedFAB
icon={'plus'}
label={'Label'}
extended={false}
visible
style={{right: 16, bottom: 16, position: 'absolute'}}
/>
);
}
export default App;
There was a problem hiding this comment.
@lukewalczak Are you all planning fixing this warning for the AnimatedFAB? This PR resolved almost all of the warnings after upgrading to 5.5.0, with the exception of the AnimatedFAB. Is there any workaround for this at the moment? I can use patch-package until there is resolution on the matter.
DimitarNestorov
force-pushed
the
chore/button-width
branch
from
485a239 to
5f99155
Compare
This comment was marked as resolved.
This comment was marked as resolved.
| }; | ||
| } | ||
|
|
||
| const SurfaceIOS = forwardRef< |
There was a problem hiding this comment.
Not a biggy but since we have MD2Surface MD3Surface SurfaceIOS and inlined logic for android and web - maybe we could extract each of them to a separate file
DimitarNestorov
force-pushed
the
chore/button-width
branch
from
5f99155 to
019d96a
Compare
src/components/Surface.tsx
Outdated
| const [filteredStyle, marginStyle, borderRadiusStyle] = splitStyles( | ||
| restStyle, | ||
| (style) => style.startsWith('margin'), | ||
| (style) => style.startsWith('border') && style.endsWith('Radius') | ||
| ); |
There was a problem hiding this comment.
React Native team wants to deprecate StyleSheet.flatten so it's something we should keep in mind when writing new code and avoid such deep manipulation of styles.
DimitarNestorov
force-pushed
the
chore/button-width
branch
from
019d96a to
b358ba0
Compare
…und with shadows
DimitarNestorov
force-pushed
the
chore/button-width
branch
from
b358ba0 to
a919300
Compare
There was a problem hiding this comment.
Tested manually on the Android and web (on both MD generations) to confirm if there is no regression related to the previously fixed issue with different custom border radius values along with custom size - tests didn't present any problem!
Code was already reviewed by other contributors, comments were addressed.
Summary
Fixes #3733
Fixes #3593
Closes #3681
Changes in this PR:
Surfacework fine with percentage widths and heights on iOSSurfacefrom 3Animated.Views to 2SurfaceiOS logic in a separate componentAnimated.View:flexwidthheightopacitytranslatemarginsborderRadiuses andbackgroundColorto bothAnimated.ViewsAffected components:
AppbarBottomNavigationBarButtonCardChipAnimatedFABFABIconButtonMenuSurfaceSnackbarSearchbarModalBannerTest plan
Percentage width and height is applied correctly. Everything else looks as before.