feat: bottom sheet component by loido · Pull Request #45 · saigontechnology/rn-base-component

loido · 2023-09-22T06:14:39Z

Summary

Features

Bugs

Check List

Your code builds clean without any errors or warnings.
Check coding convention.
Updated Storybook components and stories to reflect any changes made to the UI.
Test covers all edge cases and coverage is greater than or equal to 80%.

Proof of Completeness

OR Video link IOS

Screen.Recording.2023-09-25.at.10.26.49.mov

ANDROID

Screen.Recording.2023-09-25.at.10.28.16.mov

hangnguyensaigontech · 2023-09-22T07:01:34Z

src/components/BottomSheet/BottomSheet.tsx

+  position: 'absolute',
+}))
+
+const BottomSheetComponent = (


Where is type for Props

hangnguyensaigontech · 2023-09-22T07:02:16Z

src/components/BottomSheet/BottomSheet.tsx

+        // containerHeight={_providedContainerHeight}
+        // containerOffset={animatedContainerOffset}
+        // topInset={topInset}
+        // bottomInset={bottomInset}


please refactor to remove those unused comment

this component isn't final and that comment is helpful for the next update

hangnguyensaigontech · 2023-09-22T07:02:32Z

src/components/BottomSheet/BottomSheet.tsx

+    return animatedContainerTranslateY.value - animatedKeyboardHeight.value
+  }, [])
+
+  // const animatedSnapPoints = useNormalizedSnapPoints(


same as above

hangnguyensaigontech · 2023-09-22T07:08:21Z

example/src/stories/BottomSheet.stories.tsx

+
+  return (
+    <>
+      <Button text="wefiowejfi" onPress={() => ref?.current?.open?.()} />


I know the text wefiowejfi is just dummy text to display storybook but can we change the text for more clearly and and relate to bottom sheet?

oke, updated

hangnguyensaigontech · 2023-09-22T07:11:26Z

src/components/BottomSheet/BottomSheetContainer/BottomSheetContainer.tsx

+  detached,
+  style,
+  children,
+}: BottomSheetContainerProps) => {


Please change to use this config instead because we already agreed on it
const Component: React.FC<ComponentProps> = ({}) => {}

hangnguyensaigontech · 2023-09-22T07:13:56Z

src/components/BottomSheet/BottomSheetContent/BottomSheetContent.tsx

+import {View, Text} from 'react-native'
+import React from 'react'
+
+const BottomSheetContent = () => {


Do we need this one? If not please remove it

honghoangsts · 2023-09-29T06:51:05Z

example/package.json

    "styled-components": "^5.3.9",
    "react-native-gesture-handler": "^2.9.0",
-    "react-native-reanimated": "^3.0.2",
+    "react-native-reanimated": "3.0.2",


Why we need react-native-reanimated as a dev dependencies?

honghoangsts · 2023-09-29T06:53:37Z

example/src/stories/BottomSheet.stories.tsx

+  component: BottomSheet,
+} as ComponentMeta<typeof BottomSheet>
+
+export const Basic: ComponentStory<typeof BottomSheet> = args => {


Please use rest instead of args

example/src/stories/BottomSheet.stories.tsx

honghoangsts · 2023-09-29T06:55:00Z

package.json

    "react-native": "0.71.4",
    "react-native-builder-bob": "^0.20.4",
+    "react-native-gesture-handler": "^2.13.1",
+    "react-native-reanimated": "^3.5.4",


Why we need this lib in dev dependencies?

because i'll get an error 'not found lib'. if i don't add 2 libs in dev

honghoangsts · 2023-09-29T06:56:30Z

src/components/BottomSheet/BottomSheet.tsx

+      [animatedIsVisible, handleClose, handleOpen],
+    )
+
+    useImperativeHandle(ref, () => contextValues)


Why we need export contextValues?

because we can use this context inside and we can custom component outside

I don't get what you mean. can you give some example?

honghoangsts · 2023-09-29T07:00:13Z

src/components/BottomSheet/BottomSheetContent/BottomSheetContent.tsx

+  padding: theme?.sizes?.petite,
+}))
+
+const BottomSheetContentComponent: React.FC<BottomSheetContentProps> = ({style, children}) => (


why we need BottomSheetContentComponent? Can we use direct Container?

because BottomSheetContent only show content. BottomSheetContentContainer has calculate content height

I mean Container not BottomSheetContentContainer. because the implement for BottomSheetContentComponent just render the Container

okay we will talk about it at meeting

src/components/BottomSheet/BottomSheetContentContainer/BottomSheetContentContainer.tsx

src/components/BottomSheet/BottomSheetHeader/BottomSheetHeader.tsx

honghoangsts · 2023-09-29T07:02:11Z

src/components/BottomSheet/BottomSheetHeader/BottomSheetHeader.tsx

+
+  return (
+    <Container style={style}>
+      <Title style={animatedTitleStyle} numberOfLines={1}>


Title use the Text component

Text component it's not support animated

what is animation you apply for this title?

honghoangsts · 2023-09-29T07:04:03Z

src/components/BottomSheet/types.ts

+import type {ViewStyle} from 'react-native'
+import type {BottomSheetContextType} from './contexts/BottomSheetProvider'
+
+export interface BottomSheetProps {


There are lots of type you put inside the component so why this type you create new file?

because a type file it's help us clean code in that component

We should make it consistent. All the component that we already implemented don't have type.ts

honghoangsts · 2023-09-29T07:51:31Z

src/components/BottomSheet/BottomSheetHeader/BottomSheetHeader.tsx

+
+  return (
+    <Container style={style}>
+      <Title style={animatedTitleStyle} numberOfLines={1}>


what is animation you apply for this title?

honghoangsts · 2023-09-29T07:53:03Z

src/components/BottomSheet/BottomSheet.tsx

+      [animatedIsVisible, handleClose, handleOpen],
+    )
+
+    useImperativeHandle(ref, () => contextValues)


I don't get what you mean. can you give some example?

src/components/BottomSheet/hooks/useBottomSheet.ts

src/components/BottomSheet/components/BottomSheetHeader.tsx

src/components/BottomSheet/components/BottomSheetContainer.tsx

src/components/BottomSheet/components/BottomSheetBackdrop.tsx

example/package.json

src/components/BottomSheet/components/BottomSheetBackdrop.tsx

src/components/BottomSheet/components/BottomSheetContainer.tsx

src/components/BottomSheet/components/BottomSheetContentContainer.tsx

feat: implement bottom sheet component

6b9ea07

loido requested review from ThinhKimVo, anhle10051996, baonguyenhsts, hoangSTS, honghoangsts, huydosgtech, loc-nguyenthien and vunguyendhSTS as code owners September 22, 2023 06:14

hangnguyensaigontech reviewed Sep 22, 2023

View reviewed changes

lyderdev added 2 commits September 22, 2023 14:54

fix: update text for example bottom sheet component

6e2aa2a

feat: update bottom sheet component

7e55da2

loido requested review from ledutu2 and ngochuyduong September 26, 2023 03:10

ledutu2 approved these changes Sep 26, 2023

View reviewed changes

honghoangsts requested changes Sep 29, 2023

View reviewed changes

lyderdev added 4 commits September 29, 2023 14:24

fix: use component button in header bottom shet

dc7b8d7

fix: usetheme in content container

0f6f660

fix: rest instead of args

bea67e8

fix: rest instead of args

6e6a61c

honghoangsts requested changes Sep 29, 2023

View reviewed changes

lyderdev added 2 commits September 29, 2023 15:00

fix: refactor

0d32cb3

fix: write todo in BottomSheetContent and remove lib in devdeps

85eb6a9

baonguyenhsts suggested changes Oct 6, 2023

View reviewed changes

fix: export to index.ts for hooks and contexts

480c6f6

fix: update ver libs

5e0b544

loc-nguyenthien requested changes Oct 6, 2023

View reviewed changes

fix: add flatten style

3713e84

Conversation

loido commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Features

Bugs

Check List

Proof of Completeness

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

loido commented Sep 22, 2023 •

edited

Loading