Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Bonus Pagamenti Digitali): [#175253344] Create BottomSheet component with context provider and HOC #2306

Closed
wants to merge 22 commits into from

Conversation

CrisTofani
Copy link
Contributor

Short description

This PR adds a new BottomSheet component that can be used through any screen with the HOC withBottomSheetContext

How to test

To test request a new Bonus Vacanze and tap on QR code footer button, the previously implemented modal box is now been converted to Common BottomSheet

@pagopa-github-bot pagopa-github-bot changed the title [#175253344] Create BottomSheet component with context provider and HOC feat(Bonus Pagamenti Digitali): [#175253344] Create BottomSheet component with context provider and HOC Oct 20, 2020
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Oct 20, 2020

Affected stories

  • 🌟 #175253344: Come DEV voglio avere un componente generico per un bottom sheet

New dependencies added: react-native-reanimated and reanimated-bottom-sheet.

react-native-reanimated

Author: Krzysztof Magiera

Description: More powerful alternative to Animated library for React Native.

Homepage: https://github.com/software-mansion/react-native-reanimated#readme

Createdover 2 years ago
Last Updatedabout 1 month ago
LicenseMIT
Maintainers3
Releases42
Direct Dependenciesfbjs

reanimated-bottom-sheet

Author: Michał Osadnik

Description: Bottom sheet component

Homepage: https://github.com/osdnk/reanimated-bottom-sheet#readme

Createdover 1 year ago
Last Updated2 months ago
LicenseMIT
Maintainers1
Releases22
Keywordsreact-native, bottom-sheet, reanimated, gesture and handler
README

Reanimated Bottom Sheet

Highly configurable component imitating native bottom sheet behavior, with fully native 60 FPS animations!

Built from scratch with react-native-gesture-handler and react-native-reanimated.

Usable with Expo with no extra native dependencies!

Installation

Open a Terminal in the project root and run:

yarn add reanimated-bottom-sheet

Or if you use npm:

npm install reanimated-bottom-sheet

Now we need to install react-native-gesture-handler and react-native-reanimated.

If you are using Expo, to ensure that you get the compatible versions of the libraries, run:

expo install react-native-gesture-handler react-native-reanimated

If you are not using Expo, run the following:

yarn add react-native-reanimated react-native-gesture-handler

Or if you use npm:

npm install react-native-reanimated react-native-gesture-handler

We're done! Now you can build and run the app on your device/simulator.

Usage

import * as React from 'react';
import { StyleSheet, Text, View, Button } from 'react-native';
import Animated from 'react-native-reanimated';
import BottomSheet from 'reanimated-bottom-sheet';

export default function App() {
  const renderContent = () => (
    <View
      style={{
        backgroundColor: 'white',
        padding: 16,
        height: 450,
      }}
    >
      <Text>Swipe down to close</Text>
    </View>
  );

  const sheetRef = React.useRef(null);

  return (
    <>
      <View
        style={{
          flex: 1,
          backgroundColor: 'papayawhip',
          alignItems: 'center',
          justifyContent: 'center',
        }}
      >
        <Button
          title="Open Bottom Sheet"
          onPress={() => sheetRef.current.snapTo(0)}
        />
      </View>
      <BottomSheet
        ref={sheetRef}
        snapPoints={[450, 300, 0]}
        borderRadius={10}
        renderContent={renderContent}
      />
    </>
  );
}

Props

name required default description
snapPoints yes E.g. [300, 200, 0]. Points for snapping of bottom sheet coomponent. They define distance from bottom of the screen. Might be number or percent (as string e.g. '20%') for points or percents of screen height from bottom. Note: Array values must be in descending order.
initialSnap no 0 Determines initial snap point of bottom sheet. The value is the index from snapPoints.
renderContent no Method for rendering scrollable content of bottom sheet.
renderHeader no Method for rendering non-scrollable header of bottom sheet.
enabledGestureInteraction no true Defines if bottom sheet could be scrollable by gesture.
enabledHeaderGestureInteraction no true Defines if bottom sheet header could be scrollable by gesture.
enabledContentGestureInteraction no true Defines if bottom sheet content could be scrollable by gesture.
enabledContentTapInteraction no true Defines whether bottom sheet content could be tapped. Note: If you use Touchable* components inside your renderContent, you'll have to switch this to false to make handlers like onPress work. (See this comment.)
enabledManualSnapping no true If false blocks snapping using snapTo method.
enabledBottomClamp no false If true block movement is clamped from bottom to minimal snapPoint.
enabledBottomInitialAnimation no false If true sheet will grows up from bottom to initial snapPoint.
enabledInnerScrolling no true Defines whether it's possible to scroll inner content of bottom sheet.
callbackNode no reanimated node which holds position of bottom sheet, where 0 it the highest snap point and 1 is the lowest.
contentPosition no reanimated node which holds position of bottom sheet's content (in dp)
headerPosition no reanimated node which holds position of bottom sheet's header (in dp)
overdragResistanceFactor no 0 `Defines how violently sheet has to stopped while overdragging. 0 means no overdrag
springConfig no { } Overrides config for spring animation
innerGestureHandlerRefs no Refs for gesture handlers used for building bottom sheet. The array consists fo three refs. The first for PanGH used for inner content scrolling. The second for PanGH used for header. The third for TapGH used for stopping scrolling the content.
simultaneousHandlers no Accepts a react ref object or an array of refs to handler components.
onOpenStart no Accepts a function to be called when the bottom sheet starts to open.
onOpenEnd no Accepts a function to be called when the bottom sheet is almost fully openned.
onCloseStart no Accepts a function to be called when the bottom sheet starts to close.
onCloseEnd no Accepts a function to be called when the bottom sheet is almost closing.
callbackThreshold no 0.01 Accepts a float value from 0 to 1 indicating the percentage (of the gesture movement) when the callbacks are gonna be called.
borderRadius no Border radius of content wrapper (excluding header)

Methods

snapTo(index)

Imperative method on for snapping to snap point in given index. E.g.

// Snap to the snap point at index 0 (e.g. 450 in [450, 300, 0])
this.bottomSheetRef.current.snapTo(0)

Here this.bottomSheetRef refers to the ref passed to the BottomSheet component.

Example

More complex examples can be found in the Example folder. To view the examples in the Expo app, open a Terminal and run:

yarn
yarn prepare
cd Example
yarn
expo start

The example app is also available on Expo.

Todo

It's not finished and some work has to be done yet.

  1. Play with magic config values
  2. Horizontal mode
  3. Deal with GH in inner scrollView
  4. Cleanup code (e.g. measuring of components)

Contributing

Publishing a release

We use release-it to automate our release. If you have publish access to the NPM package, run the following from the master branch to publish a new release:

yarn release

NOTE: You must have a GITHUB_TOKEN environment variable available. You can create a GitHub access token with the "repo" access here.

Generated by 🚫 dangerJS against 756ec0e

@codecov-io
Copy link

codecov-io commented Oct 20, 2020

Codecov Report

Merging #2306 into master will decrease coverage by 0.04%.
The diff coverage is 37.20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2306      +/-   ##
==========================================
- Coverage   48.05%   48.00%   -0.05%     
==========================================
  Files         588      591       +3     
  Lines       16857    16930      +73     
  Branches     3065     3350     +285     
==========================================
+ Hits         8101     8128      +27     
- Misses       8714     8760      +46     
  Partials       42       42              
Impacted Files Coverage Δ
ts/components/BottomSheetComponent.tsx 29.62% <29.62%> (ø)
ts/components/ui/BottomSheet.tsx 33.33% <33.33%> (ø)
...tures/bonus/bonusVacanze/components/QrModalBox.tsx 61.90% <33.33%> (+6.73%) ⬆️
...s/bonus/bonusVacanze/screens/ActiveBonusScreen.tsx 30.23% <50.00%> (ø)
ts/components/helpers/withBottomSheetContext.tsx 88.88% <88.88%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1705c39...756ec0e. Read the comment docs.

@CrisTofani CrisTofani marked this pull request as draft October 20, 2020 12:54
@CrisTofani CrisTofani marked this pull request as ready for review October 20, 2020 14:29
Comment on lines -719 to +720
)(withLightModalContext(ActiveBonusScreen));
)(withBottomSheetContext(withLightModalContext(ActiveBonusScreen)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lgtm, but being a functional component we could also use:

const context = useContext(BottomSheetContext);
[...]
context.showBS(modalBox, I18n.t("bonus.bonusVacanze.name"), 466);

without withBottomSheetContext and BottomSheetContextInterface to inject the context.
I found this out just now and it seemed nice to share!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriziofff nice suggestion! I noticed the useContext hook, but actually didn't focused on how to use it.

I'm not actually sure if using 2 different approaches may result too chaotic. We may use at the same time useContext hook for LightModal too, but this will create a misalignment with the rest of the codebase.

I, anyway, prefer the usage of the hook.

Comment on lines 16 to 26
class WithBottomSheetContext extends React.Component<P> {
public render() {
return (
<BottomSheetConsumer>
{contextProps => (
<WrappedComponent {...contextProps} {...this.props} />
)}
</BottomSheetConsumer>
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My proposal to avoid a class component.

Suggested change
class WithBottomSheetContext extends React.Component<P> {
public render() {
return (
<BottomSheetConsumer>
{contextProps => (
<WrappedComponent {...contextProps} {...this.props} />
)}
</BottomSheetConsumer>
);
}
}
const WithBottomSheetContext: React.FunctionComponent<P> = props => (
<BottomSheetConsumer>
{contextProps => <WrappedComponent {...contextProps} {...props} />}
</BottomSheetConsumer>
);

Comment on lines 13 to 18
showBS: (
component: React.ReactNode,
title: string,
snapPoint: number
) => void;
hideBS: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you think about call the two methods showBottomSheet and hideBottomSheet in order to have a self-explanatory name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Will rename both functions

) : (
<>
<Animated.View
pointerEvents={"none"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the bottomsheet is open is still possible to interact with the background (the "dark area"). Maybe should be "auto" when is opened?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriziofff Good catch! This actually was my third version of the implementation and I started anytime from the beginning, I forgot to remove the pointerEvents attribute!

Comment on lines 115 to 122
const closeBottomSheet = () => {
setAccessible(false);
if (bottomSheetRef && bottomSheetRef.current) {
bottomSheetRef.current.snapTo(0);
closeBackdropAnimation();
props.handleClose();
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const closeBottomSheet = () => {
setAccessible(false);
if (bottomSheetRef && bottomSheetRef.current) {
bottomSheetRef.current.snapTo(0);
closeBackdropAnimation();
props.handleClose();
}
};
const closeBottomSheet = () => {
setAccessible(false);
if (bottomSheetRef && bottomSheetRef.current) {
bottomSheetRef.current.snapTo(0);
}
};

When the bottomsheet is closed with the X there is a slightly different graphical behavior.
It doesn't do the exit animation and when it is called next time it does not do the entry animation.

Removing these lines works for me, because the backdropanimation is executed by the callback, but I don't know if removing props.handleClose(); generate any others side effects.

Comment on lines 174 to 176
renderContent={() => (
<View style={styles.content}>{props.content}</View>
)}
Copy link
Contributor

@fabriziofff fabriziofff Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
renderContent={() => (
<View style={styles.content}>{props.content}</View>
)}
renderContent={() => (
<View style={styles.content}>
<SafeAreaView style={IOStyles.flex}>{props.content}</SafeAreaView>
</View>
)}

My proposal to avoid possible overflows (like this) in the safe area:

…mponent' into 175253344-create-bottom-sheet-component
@CrisTofani CrisTofani closed this Nov 3, 2020
@CrisTofani CrisTofani deleted the 175253344-create-bottom-sheet-component branch November 3, 2020 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants