-
Notifications
You must be signed in to change notification settings - Fork 4
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: Pressable Group Updates #927
base: release/2.0.7
Are you sure you want to change the base?
Changes from 5 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import { ITextProps } from "@design-system/Text"; | ||
import { ITextStyleProps } from "@design-system/Text/Text.props"; | ||
import { ParseShape } from "react-native-parsed-text"; | ||
|
||
export interface IParsedTextProps extends ITextProps { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we're extending our text props but the component rendered is not our Text component? Maybe I'm missing something but I don't think it's a good pattern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that the lib as |
||
parse: ParseShape[]; | ||
pressableStyle?: ITextStyleProps; | ||
} | ||
Comment on lines
+1
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we shouldn't create a new file for props if they are small |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { getTextStyle } from "@design-system/Text/Text.utils"; | ||
import { useAppTheme } from "@theme/useAppTheme"; | ||
import React, { forwardRef, memo, useMemo } from "react"; | ||
import RNParsedText from "react-native-parsed-text"; | ||
|
||
import { IParsedTextProps } from "./ParsedText.props"; | ||
|
||
const ParsedTextInner = forwardRef<RNParsedText, IParsedTextProps>( | ||
(props, ref) => { | ||
const { themed } = useAppTheme(); | ||
const styles = getTextStyle(themed, props); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. space please 🙏 |
||
const childThemedProps = useMemo(() => { | ||
return { | ||
...props, | ||
...props.pressableStyle, | ||
}; | ||
}, [props]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useMemo for something like this is overkill and even worst than not using it. Since we pass props in deps array. I would just do const childThemedProps = { ...props, ...props.pressableStyle } |
||
const pressableStyles = getTextStyle(themed, childThemedProps); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. space please 🙏 |
||
const parseOptions = useMemo( | ||
() => | ||
props.parse.map(({ onPress, ...rest }) => ({ | ||
...rest, | ||
onPress, | ||
style: pressableStyles, | ||
})), | ||
[props.parse, pressableStyles] | ||
); | ||
|
||
return ( | ||
<RNParsedText {...props} parse={parseOptions} ref={ref} style={styles} /> | ||
); | ||
} | ||
); | ||
|
||
export const ParsedText = memo(ParsedTextInner); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we can put all inline like this: export const ParsedText = memo( |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import { ITextStyleProps } from "@design-system/Text/Text.props"; | ||
import { memo, useCallback, useMemo } from "react"; | ||
|
||
import { ParsedText } from "./ParsedText/ParsedText"; | ||
|
||
const pressableStyle: ITextStyleProps = { | ||
weight: "bold", | ||
}; | ||
|
||
const PressableProfileWithTextInner = ({ | ||
profileAddress, | ||
profileDisplay, | ||
text, | ||
onPress, | ||
}: { | ||
onPress: (address: string) => void; | ||
text: string; | ||
profileDisplay: string; | ||
profileAddress: string; | ||
}) => { | ||
const handlePress = useCallback(() => { | ||
return onPress(profileAddress); | ||
}, [profileAddress, onPress]); | ||
|
||
const pattern = useMemo( | ||
() => new RegExp(profileDisplay, "g"), | ||
[profileDisplay] | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I love having a space between things |
||
const parseOptions = useMemo( | ||
() => [ | ||
{ | ||
onPress: handlePress, | ||
pattern, | ||
}, | ||
], | ||
[handlePress, pattern] | ||
); | ||
|
||
return ( | ||
<ParsedText | ||
preset="subheading" | ||
size="xxs" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. having |
||
pressableStyle={pressableStyle} | ||
parse={parseOptions} | ||
> | ||
{text} | ||
</ParsedText> | ||
); | ||
}; | ||
|
||
export const PressableProfileWithText = memo(PressableProfileWithTextInner); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
import { PressableProfileWithText } from "@components/PressableProfileWithText"; | ||
import { render, fireEvent } from "@testing-library/react-native"; | ||
import { navigate } from "@utils/navigation"; | ||
|
||
// Mock the navigate function from @utils/navigation | ||
jest.mock("@utils/navigation", () => ({ | ||
navigate: jest.fn(), | ||
})); | ||
|
||
describe("PressableProfileWithTextInner", () => { | ||
const profileAddress = "0x123"; | ||
const profileDisplay = "User123"; | ||
const text = "Hello User123, welcome!"; | ||
const textStyle = { fontSize: 18 }; | ||
const pressableTextStyle = { fontSize: 18, color: "blue" }; | ||
|
||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it("renders ParsedText with the correct text and style", () => { | ||
const { getByText } = render( | ||
<PressableProfileWithText | ||
profileAddress={profileAddress} | ||
profileDisplay={profileDisplay} | ||
text={text} | ||
textStyle={textStyle} | ||
Check failure on line 27 in components/__tests__/PressableProfileWithText.test.tsx GitHub Actions / tsc
|
||
/> | ||
); | ||
|
||
const renderedText = getByText(text); | ||
expect(renderedText).toBeTruthy(); | ||
expect(renderedText.props.style).toEqual(textStyle); | ||
}); | ||
|
||
it("calls navigate to profile on profileDisplay press", () => { | ||
const { getByText } = render( | ||
<PressableProfileWithText | ||
profileAddress={profileAddress} | ||
profileDisplay={profileDisplay} | ||
text={text} | ||
textStyle={textStyle} | ||
Check failure on line 42 in components/__tests__/PressableProfileWithText.test.tsx GitHub Actions / tsc
|
||
/> | ||
); | ||
|
||
const parsedText = getByText(profileDisplay); | ||
|
||
// Simulate the press on the profileDisplay text | ||
fireEvent.press(parsedText); | ||
|
||
expect(navigate).toHaveBeenCalledWith("Profile", { | ||
address: profileAddress, | ||
}); | ||
}); | ||
|
||
it("does not call navigate when profileAddress is undefined", () => { | ||
const { getByText } = render( | ||
<PressableProfileWithText | ||
profileAddress={undefined as any} | ||
profileDisplay={profileDisplay} | ||
text={text} | ||
textStyle={textStyle} | ||
Check failure on line 62 in components/__tests__/PressableProfileWithText.test.tsx GitHub Actions / tsc
|
||
/> | ||
); | ||
|
||
const parsedText = getByText(profileDisplay); | ||
|
||
// Simulate the press on the profileDisplay text | ||
fireEvent.press(parsedText); | ||
|
||
expect(navigate).not.toHaveBeenCalled(); | ||
}); | ||
|
||
it("renders with the provided style", () => { | ||
const { getByText } = render( | ||
<PressableProfileWithText | ||
profileAddress={profileAddress} | ||
profileDisplay={profileDisplay} | ||
text={text} | ||
textStyle={textStyle} | ||
Check failure on line 80 in components/__tests__/PressableProfileWithText.test.tsx GitHub Actions / tsc
|
||
/> | ||
); | ||
|
||
const parsedText = getByText(profileDisplay); | ||
|
||
expect(parsedText.props.style).toEqual([textStyle, undefined]); | ||
}); | ||
|
||
it("renders with the provided style for pressable text", () => { | ||
const { getByText } = render( | ||
<PressableProfileWithText | ||
profileAddress={profileAddress} | ||
profileDisplay={profileDisplay} | ||
text={text} | ||
textStyle={textStyle} | ||
Check failure on line 95 in components/__tests__/PressableProfileWithText.test.tsx GitHub Actions / tsc
|
||
pressableTextStyle={pressableTextStyle} | ||
/> | ||
); | ||
|
||
const parsedText = getByText(profileDisplay); | ||
|
||
expect(parsedText.props.style).toEqual([textStyle, pressableTextStyle]); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used? 😮
I checked because we have a weight props for Text so we can just use this instead