Fader - a component to easily add fade look to other components #847
Conversation
src/assets/images/index.js
Outdated
| @@ -1,5 +1,18 @@ | |||
| export const images = { | |||
| // TODO: deprecate | |||
There was a problem hiding this comment.
Since the gradient image can changed according to many different requirements I think it's better that the user will provide the image instead.
If we want to provide defaults we can keep these images, but keep them as part of the component assets and don't expose them in our Assets
There was a problem hiding this comment.
Moved internally
| import React, {useCallback} from 'react'; | ||
| import { | ||
| StyleSheet, | ||
| // eslint-disable-next-line no-unused-vars |
There was a problem hiding this comment.
It's impossible to read the code with all the lint comments.
First, there's a way to ignore a block of code with eslint.
Second, because of TS, this lint error won't break the build for now, I know it's annoying to see the red marks but for now ignore them, these things will be fixed/handled all together once we complete the TS migration.
| /** | ||
| * Use to render the fade | ||
| */ | ||
| renderFader: () => React.ReactNode; |
There was a problem hiding this comment.
If the whole idea of this HOC is to provide a render function that renders a component based on props. wouldn't it be easier to create a component?
I think it would be a lot simpler if the use can just render a "ScrollFader" component. WDYT?
There was a problem hiding this comment.
Maybe if there has to be a single child that has to have onScroll (or has to be a supportedView).
There was a problem hiding this comment.
Changed to Fader, changed the title (but not the branch name :) )
# Conflicts: # generatedTypes/index.d.ts
M-i-k-e-l
changed the title
src/components/fader/index.tsx
Outdated
| const isVisible = useCallback(() => { | ||
| return props.visible || _.isUndefined(props.visible); | ||
| }, [props.visible]); |
There was a problem hiding this comment.
Do we need the callback?
Can't we check directly props.visible in the render?
src/components/fader/index.tsx
Outdated
| /** | ||
| * The location of the fader (the image is different), defaults to Fader.location.RIGHT | ||
| */ | ||
| location?: FaderLocation; |
There was a problem hiding this comment.
What do you think of renaming to position? to be more consistent with other components (toast position, badge position, etc..)
| @@ -0,0 +1,89 @@ | |||
| import _ from 'lodash'; | |||
There was a problem hiding this comment.
Looks great. Very simple and easy to understand the component 👍
Notes about the images:
gradientimage and rotating it, I only managed doing it withtransformand that requiredonLayoutso I'd know the size of the view, this worked all right on iOS but was slower on Android for some reason - it did not look good enough, so I chose to go with 4 images for the different orientations.gradientimage in the next version.