-
Notifications
You must be signed in to change notification settings - Fork 5
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
New pricing page #461
New pricing page #461
Conversation
…to Brenosalv/feature/431-pricing-page-updates
…dy is the default value
…om/estuary/marketing-site into Brenosalv/feature/431-pricing-page-updates
Visit the preview URL for this PR (updated for commit 26639d8): https://estuary-marketing--pr461-brenosalv-feature-43-70hb9hrf.web.app (expires Fri, 27 Sep 2024 21:21:33 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 76f6b095a0752e5d9c6c890267f9fdc3e392161e |
…etter performance and UX
…to Brenosalv/feature/431-pricing-page-updates
src/components/Carousel/index.tsx
Outdated
backgroundColor: | ||
selectedIndex === index | ||
? '#5072EB' | ||
: '#FFFFFF', | ||
color: | ||
selectedIndex === index | ||
? '#5072EB' | ||
: '#FFFFFF', | ||
borderColor: | ||
selectedIndex === index | ||
? '#5072EB' | ||
: '#47506D', | ||
}} |
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.
To reduce dynamic styling I think we should just mark the li
as selected with a class name.
@@ -30,17 +30,30 @@ const SectionThree = () => { | |||
} | |||
`); | |||
|
|||
const cardsPerSlide = 3; |
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.
This should be declared outside outside the function.
) : null} | ||
</div> | ||
<p className={planDescriptionStyle}>{description}</p> | ||
<div className={divider} /> |
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.
Can you try to make this just use a hr
tag?
) => { | ||
const inputValue = event.target.value; | ||
|
||
if (/^\d*$/.test(inputValue)) { |
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.
Please store this regex in a const
outside of the component.
export const PricingCalculator = ({ | ||
isDarkTheme = false, | ||
}: PricingCalculatorProps) => { | ||
const [selectedGbs, setSelectedGbs] = React.useState(1); |
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.
This applies to a few places here... can you import the hook you're using and not all of React.
…to Brenosalv/feature/431-pricing-page-updates
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.
lgtm
Adding a minimum value
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.
lgtm
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.
lgtm
#431
Changes