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.

Sign up for GitHub

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

Jump to bottom

fix: Buyer guarantee refactor QA fixes #8440

Merged
merged 4 commits into from
Sep 24, 2021
Merged

fix: Buyer guarantee refactor QA fixes #8440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3302822
Fixed links not working on mobile
jo-rs Sep 21, 2021
2eeb85c
Condition fix
jo-rs Sep 21, 2021
bc440c9
CTA fixes
jo-rs Sep 21, 2021
2194166
Addressed responsive issues
jo-rs Sep 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 49 additions & 13 deletions src/v2/Apps/BuyerGuarantee/Components/Feature.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,22 @@ import {
Flex,
IconProps,
Text,
themeProps,
} from "@artsy/palette"
import { Media } from "v2/Utils/Responsive"
import { useMatchMedia } from "v2/Utils/Hooks/useMatchMedia"

interface ConditionalWrapperProps {
condition: boolean
wrapper: (children: React.ReactElement) => JSX.Element
children: React.ReactElement
}

const ConditionalWrapper: React.FC<ConditionalWrapperProps> = ({
condition,
wrapper,
children,
}) => (condition ? wrapper(children) : children)
Comment on lines +14 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Conditional wrapper to avoid duplicate code


interface FeatureProps {
title: string
Expand All @@ -24,6 +38,7 @@ export const Feature: React.FC<FeatureProps> = ({
icon,
onClick,
}) => {
const isMobile = useMatchMedia(themeProps.mediaQueries.xs)
const Icon = icon
const learnMore = (
<Flex pt={2} justifyContent="center">
Expand All @@ -47,19 +62,40 @@ export const Feature: React.FC<FeatureProps> = ({
<Box>
<Icon height={60} width={60} />
</Box>
<Flex flexDirection={["row", "column"]} alignItems="center">
<Text fontWeight="bold" my={2} variant="sm">
{title}
</Text>
<Media greaterThan="xs">
<Text variant="xs">{text}</Text>
{!!forcedSecondLine && <Text variant="xs">{forcedSecondLine}</Text>}
{!!onClick && <Box>{learnMore}</Box>}
</Media>
<Media lessThan="sm">
{!!onClick && <ArrowRightIcon height="20px" width="20px" my={2} />}
</Media>
</Flex>
<ConditionalWrapper
condition={isMobile! && onClick}
wrapper={children => (
<Button
inline
variant="noOutline"
onClick={onClick}
style={{ whiteSpace: "pre-wrap" }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bonus: addresses links not wrapping and overlapping on smaller screens

>
{children}
</Button>
)}
>
<Flex flexDirection={["row", "column"]} alignItems="center">
<Text fontWeight="bold" my={2} variant="sm">
{title}
</Text>
<Media greaterThan="xs">
<Text variant="xs">{text}</Text>
{!!forcedSecondLine && <Text variant="xs">{forcedSecondLine}</Text>}
{!!onClick && <Box>{learnMore}</Box>}
</Media>
<Media lessThan="sm">
{!!onClick && (
<ArrowRightIcon
height="20px"
width="20px"
my={2}
mr={isMobile ? "-20px" : 0}
/>
)}
</Media>
</Flex>
</ConditionalWrapper>
</Flex>
)
}
6 changes: 3 additions & 3 deletions src/v2/Apps/BuyerGuarantee/Routes/BuyerGuaranteeIndex.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ export const BuyerGuaranteeIndex: React.FC = () => {
}

const learnMoreButton = (width?: string) => (
<RouterLink to={supportArticleURL} target="_blank">
<RouterLink noUnderline to={supportArticleURL} target="_blank">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing underline

<Button
width={width ? width : ["100%", "80%", "50%"]}
variant="secondaryOutline"
variant={isMobile ? "secondaryOutline" : "primaryBlack"}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filled in CTA for desktop

>
Learn More
</Button>
Expand Down Expand Up @@ -229,7 +229,7 @@ export const BuyerGuaranteeIndex: React.FC = () => {
</Flex>
</Flex>
{/* Artsy Guarantee Sections desktop */}
<GridColumns gridColumnGap={4} gridRowGap={[6, 12]} my={[6, 12]}>
<GridColumns gridColumnGap={[0, 4]} gridRowGap={[6, 12]} my={[6, 12]}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses responsive cropping

<Column span={6}>
{authenticityGuaranteeImageURL && (
<Image
Expand Down