-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Website: Implement glowing cursor effect #1009
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of changes focused on enhancing the UI components, particularly around glow hover effects and newsletter functionality. The changes include new components like Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
ui/src/components/use-glow-hover/glow-hover-effect.ts (1)
124-124
: Avoid assignments within arrow function expressionsUsing assignments within arrow function expressions without braces can be confusing and reduce code readability.
Apply the following diff to improve clarity:
-onIdUpdate: (newId) => (lightSizeEnterAnimationId = newId), +onIdUpdate: (newId) => { + lightSizeEnterAnimationId = newId; +},Repeat this change for lines 158, 177, and 196.
Also applies to: 158-158, 177-177, 196-196
🧰 Tools
🪛 Biome (1.9.4)
[error] 124-124: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
ui/src/components/use-glow-hover/use-glow-hover.ts (1)
13-13
: OptimizeuseEffect
dependencies to prevent unnecessary re-rendersUsing
Object.values(options)
in the dependency array may cause the effect to run on every render ifoptions
is not memoized, leading to performance issues.Consider memoizing the
options
object or explicitly listing each option in the dependency array.ui/src/components/use-glow-hover/linear-animation.ts (1)
14-15
: ClampinitialProgress
to the range [0, 1]Currently,
initialProgress
can be outside the range[0, 1]
, which may cause unexpected behavior in the animation.Apply this diff to ensure
initialProgress
is within the valid range:}: LinearAnimationParams) => { + initialProgress = Math.min(Math.max(initialProgress, 0), 1); if (time === 0) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ui/src/components/containers/base-container.tsx
(1 hunks)ui/src/components/use-glow-hover/glow-hover-effect.ts
(1 hunks)ui/src/components/use-glow-hover/index.ts
(1 hunks)ui/src/components/use-glow-hover/linear-animation.ts
(1 hunks)ui/src/components/use-glow-hover/use-glow-hover.ts
(1 hunks)ui/src/index.ts
(1 hunks)website/src/app/[lang]/[region]/(website)/(home)/(sections)/overview.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- ui/src/components/use-glow-hover/index.ts
🧰 Additional context used
🪛 GitHub Actions: Website
ui/src/components/containers/base-container.tsx
[error] 14-14: Property 'wrapperClassName' does not exist on type '{ backgroundColor?: BackgroundColor | undefined; wrapperRef?: Ref | undefined; } & HTMLAttributes'
🪛 Biome (1.9.4)
ui/src/components/use-glow-hover/glow-hover-effect.ts
[error] 124-124: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 158-158: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 177-177: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 196-196: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (2)
ui/src/index.ts (1)
30-30
: LGTM!The new export follows the existing pattern and maintains alphabetical order.
website/src/app/[lang]/[region]/(website)/(home)/(sections)/overview.tsx (1)
14-26
: Remove temporary example code.The TODO comment indicates this code should be removed before merge. Please remove this example usage of GLowHoverContainer.
export const GLowHoverContainer = React.forwardRef<HTMLDivElement, Omit<BaseContainerProps, 'wrapperRef'>>( | ||
({ className, ...props }, ref) => { | ||
const refCard = useGlowHover({ lightColor: '#CEFF00' }); | ||
|
||
return ( | ||
<BaseContainer | ||
wrapperClassName={twMerge('theme-blue', className)} | ||
ref={ref} | ||
{...props} | ||
wrapperRef={refCard as React.Ref<HTMLDivElement>} | ||
/> | ||
); | ||
}, |
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.
🛠️ Refactor suggestion
Improve type safety and configuration of GLowHoverContainer.
Two suggestions for improvement:
- Avoid type casting of refCard
- Make the light color configurable
-export const GLowHoverContainer = React.forwardRef<HTMLDivElement, Omit<BaseContainerProps, 'wrapperRef'>>(
+type GlowHoverContainerProps = Omit<BaseContainerProps, 'wrapperRef'> & {
+ lightColor?: string;
+};
+
+export const GLowHoverContainer = React.forwardRef<HTMLDivElement, GlowHoverContainerProps>(
- ({ className, ...props }, ref) => {
+ ({ className, lightColor = '#CEFF00', ...props }, ref) => {
- const refCard = useGlowHover({ lightColor: '#CEFF00' });
+ const refCard = useGlowHover({ lightColor });
return (
<BaseContainer
wrapperClassName={twMerge('theme-blue', className)}
ref={ref}
{...props}
- wrapperRef={refCard as React.Ref<HTMLDivElement>}
+ wrapperRef={refCard}
/>
);
},
);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const GLowHoverContainer = React.forwardRef<HTMLDivElement, Omit<BaseContainerProps, 'wrapperRef'>>( | |
({ className, ...props }, ref) => { | |
const refCard = useGlowHover({ lightColor: '#CEFF00' }); | |
return ( | |
<BaseContainer | |
wrapperClassName={twMerge('theme-blue', className)} | |
ref={ref} | |
{...props} | |
wrapperRef={refCard as React.Ref<HTMLDivElement>} | |
/> | |
); | |
}, | |
type GlowHoverContainerProps = Omit<BaseContainerProps, 'wrapperRef'> & { | |
lightColor?: string; | |
}; | |
export const GLowHoverContainer = React.forwardRef<HTMLDivElement, GlowHoverContainerProps>( | |
({ className, lightColor = '#CEFF00', ...props }, ref) => { | |
const refCard = useGlowHover({ lightColor }); | |
return ( | |
<BaseContainer | |
wrapperClassName={twMerge('theme-blue', className)} | |
ref={ref} | |
{...props} | |
wrapperRef={refCard} | |
/> | |
); | |
}, |
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.
@ssandino hi!
Could you please tell if you need the glowing color to be configurable?
As I can see now, the blue color mostly used throughout the site to make banners and accent sections and yellow cursor glow fits perfectly here. However, there's no problem adding prop for a custom color.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
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.
According to live chat with @ssandino during the coding retreat, it's been decided, that no color customization is needed here.
<GLowHoverContainer className="mb-16 mt-8 flex flex-col items-center justify-center md:mb-48 md:mt-16"> | ||
<Typography size="3xl" weight="medium" className="my-10 text-center"> | ||
{translator.t('section-2.title-1')} | ||
</Typography> | ||
<div className="mb-8 text-center"> | ||
{translator.t<{ text: string; color?: FontColor }[]>('section-2.title-2').map((title, index) => ( | ||
<Typography as="span" size="3xl" weight="medium" color={title.color} key={index}> | ||
{title.text}{' '} | ||
</Typography> | ||
))} | ||
</div> | ||
</GLowHoverContainer> | ||
|
||
<BaseContainer className="mx-auto mb-16 mt-8 flex w-4/5 flex-col items-center justify-center md:mb-48 md:mt-16 lg:w-3/5"> | ||
<Typography size="3xl" weight="medium" className="my-10 text-center"> | ||
{translator.t('section-2.title-1')} | ||
</Typography> | ||
<div className="mb-8 text-center"> | ||
{translator.t<{ text: string; color?: FontColor }[]>('section-2.title-2').map((title, index) => ( | ||
<Typography as="span" size="3xl" weight="medium" color={title.color} key={index}> | ||
{title.text}{' '} | ||
</Typography> | ||
))} | ||
</div> | ||
<Typography size="3xl" weight="medium" className="mb-5 mt-12 text-center"> | ||
{translator.t('section-2.title-3')} | ||
</Typography> | ||
<ol className="mb-8 list-decimal"> | ||
<li> | ||
<Typography as="span">{translator.t('section-2.text-3.1')}</Typography> | ||
</li> | ||
<li> | ||
<Typography as="span">{translator.t('section-2.text-3.2')}</Typography> | ||
</li> | ||
</ol> | ||
<Typography size="3xl" weight="medium" className="mb-5 mt-12 text-center"> | ||
{translator.t('section-2.title-4')} | ||
</Typography> | ||
<Typography className="mb-8">{translator.t('section-2.text-4')}</Typography> | ||
<Typography size="3xl" weight="medium" className="mb-5 mt-12 text-center"> | ||
{translator.t('section-2.title-5')} | ||
</Typography> | ||
<Typography className="w-full">{translator.t('section-2.text-5')}</Typography> | ||
</BaseContainer> |
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.
Avoid content duplication.
The same content is duplicated between GLowHoverContainer and BaseContainer. Choose one container based on the intended design.
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.
JFI: this part of duplicated content will be removed before merge
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.
@IPogorelova Thanks for clarifying! I see the TODO comment now, and understand this is just a temporary example.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Visit the preview URL for this PR (updated for commit e103dae): https://si-admin-staging--pr1009-implement-glowing-cu-5itsz7qr.web.app (expires Sat, 01 Feb 2025 08:37:01 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
The deployment on Vercel preview throws following error:
And the co-pilot bug report says: |
… into implement-glowing-cursor-effect
…-effect-wip-fix' into sandino/implement-glowing-cursor-effect-wip-fix # Conflicts: # ui/src/components/use-glow-hover/glow-hover-effect.ts
…-effect-wip-fix' into sandino/implement-glowing-cursor-effect-wip-fix
… into implement-glowing-cursor-effect
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/components/newsletter-form/newsletter-form.tsx (1)
45-68
: Enhance form accessibility.Add aria-label to the form for better screen reader support.
<form className="flex w-full flex-col items-center justify-center gap-2 sm:flex-row" + aria-label="Newsletter subscription form" onSubmit={form.handleSubmit(onSubmit)} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/src/components/newsletter-form/newsletter-form.tsx
(1 hunks)website/src/components/newsletter-popup/newsletter-popup-client.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (4)
website/src/components/newsletter-popup/newsletter-popup-client.tsx (2)
3-3
: LGTM! Good separation of concerns.The form logic has been cleanly extracted to a dedicated NewsletterForm component.
Also applies to: 37-37
Line range hint
12-17
: LGTM! Type export enables reuse.Making NewsletterPopupTranslations exportable allows proper type sharing across components.
website/src/components/newsletter-form/newsletter-form.tsx (2)
13-17
:⚠️ Potential issueAdd proper type for Toast prop.
The Toast prop is marked optional but used without null checks on line 35.
type NewsletterForm = { lang: LanguageCode; - t?: Toast; + t: Toast; translations: NewsletterPopupTranslations; };Likely invalid or redundant comment.
28-43
: 🛠️ Refactor suggestionImprove error handling and language support.
Three issues need attention:
- Language handling is limited to 'de' and 'en'
- Mixed usage of async/await with .then()
- Missing error catch block
const onSubmit = async (values: FormSchema) => { const body: CreateNewsletterSubscription = { email: values.email, - language: lang === 'de' ? 'de' : 'en', + language: lang, }; - api.post('/api/newsletter/subscription/public', body).then((response) => { - if (response.status === 200) { - if (t.id) { - toast.dismiss(t.id); - } - toast.success(translations.toastSuccess); - } else { + try { + const response = await api.post('/api/newsletter/subscription/public', body); + if (response.status === 200) { + toast.dismiss(t.id); + toast.success(translations.toastSuccess); + } else { toast.error(translations.toastFailure); } - }); + } catch (error) { + toast.error(translations.toastFailure); + } };Likely invalid or redundant comment.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/components/newsletter-form/newsletter-form.tsx (1)
13-17
: Add specific type for translations object.Define exact shape of required translation strings for better type safety.
+type NewsletterTranslations = { + toastSuccess: string; + toastFailure: string; + emailPlaceholder: string; + buttonAddSubscriber: string; +}; type NewsletterForm = { lang: LanguageCode; t?: Toast; - translations: NewsletterPopupTranslations; + translations: NewsletterTranslations; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/src/components/newsletter-form/newsletter-form.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
website/src/components/newsletter-form/newsletter-form.tsx
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test website
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (4)
website/src/components/newsletter-form/newsletter-form.tsx (4)
19-26
: Clean form setup with proper validation.Good use of zod schema and react-hook-form configuration.
30-32
: Support all available language codes.The language selection only handles 'de' and defaults to 'en', ignoring other supported languages.
33-42
: Simplify error handling with async/await and optional chaining.The promise chain and nested conditionals can be simplified.
- api.post('/api/newsletter/subscription/public', body).then((response) => { - if (response.status === 200) { - if (t && t.id) { - toast.dismiss(t.id); - } - toast.success(translations.toastSuccess); - } else { - toast.error(translations.toastFailure); - } - }); + try { + const response = await api.post('/api/newsletter/subscription/public', body); + t?.id && toast.dismiss(t.id); + response.status === 200 + ? toast.success(translations.toastSuccess) + : toast.error(translations.toastFailure); + } catch { + toast.error(translations.toastFailure); + }🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
45-68
: Well-structured form layout with proper responsiveness.Clean implementation using FormField components and responsive styling.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
website/src/components/newsletter-glow-container/newsletter-glow-container.tsx (1)
18-27
: Consider extracting newsletter form section to a separate component.The nested structure with multiple divs could be simplified by extracting the newsletter form section into a separate component.
+const NewsletterFormSection = ({ lang, translations }: { lang: LanguageCode; translations: NewsletterPopupTranslations }) => ( + <div className="mt-8 flex w-full justify-center sm:w-full md:max-w-md"> + <NewsletterForm lang={lang} translations={translations} /> + </div> +); const NewsletterGlowContainer = ({ title, lang, formTranslations }: NewsletterGlowContainerProps) => { return ( <GlowHoverContainer> <div className="flex flex-col items-center py-12"> <div className="align-center flex flex-col"> <Typography size="2xl" color="foreground" weight="medium"> {title} </Typography> </div> - <div className="mt-8 flex w-full justify-center sm:w-full md:max-w-md"> - <NewsletterForm lang={lang} translations={formTranslations} /> - </div> + <NewsletterFormSection lang={lang} translations={formTranslations} /> </div> </GlowHoverContainer> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/src/components/newsletter-glow-container/newsletter-glow-container.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
website/src/components/newsletter-glow-container/newsletter-glow-container.tsx
[error] 9-9: Shouldn't redeclare 'NewsletterForm'. Consider to delete it or rename it.
'NewsletterForm' is defined here:
(lint/suspicious/noRedeclare)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test website
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (2)
website/src/components/newsletter-glow-container/newsletter-glow-container.tsx (2)
15-15
:⚠️ Potential issueUpdate component props type reference.
Update the component definition to use the renamed type.
-const NewsletterGlowContainer = ({ title, lang, formTranslations }: NewsletterForm) => { +const NewsletterGlowContainer = ({ title, lang, formTranslations }: NewsletterGlowContainerProps) => {Likely invalid or redundant comment.
17-28
: Verify GlowHoverContainer implementation.The component uses
GlowHoverContainer
from@socialincome/ui
. Let's verify its implementation and usage across the codebase.✅ Verification successful
GlowHoverContainer implementation is correct
The component is properly implemented and correctly used in the newsletter container. The component chain (GlowHoverContainer → BaseContainer) and all dependencies are well-structured and type-safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify GlowHoverContainer implementation and usage # Check GlowHoverContainer implementation echo "Checking GlowHoverContainer implementation..." ast-grep --pattern 'export const GlowHoverContainer = ({ children }) => { $$$ }' # Check other usages of GlowHoverContainer echo "Checking GlowHoverContainer usages..." rg -l "GlowHoverContainer"Length of output: 430
Script:
#!/bin/bash # Check the implementation details echo "=== GlowHoverContainer Implementation ===" cat ui/src/components/containers/glow-hover-container.tsx # Check for any prop type definitions echo -e "\n=== Prop Types ===" ast-grep --pattern 'interface $_ { $$$ }' ui/src/components/containers/glow-hover-container.tsxLength of output: 937
Script:
#!/bin/bash # Check BaseContainer implementation and props echo "=== BaseContainer Implementation ===" cat ui/src/components/containers/base-container.tsx echo -e "\n=== useGlowHover Hook Implementation ===" cat ui/src/components/use-glow-hover.tsxLength of output: 1024
Script:
#!/bin/bash # Search for useGlowHover hook implementation echo "=== Searching for useGlowHover hook ===" fd -e ts -e tsx | xargs rg "export.*useGlowHover"Length of output: 372
type NewsletterForm = { | ||
title: string; | ||
lang: LanguageCode; | ||
formTranslations: NewsletterPopupTranslations; | ||
}; |
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.
Rename type to avoid naming conflict.
The type NewsletterForm
conflicts with the imported component of the same name. Rename it to be more specific.
-type NewsletterForm = {
+type NewsletterGlowContainerProps = {
title: string;
lang: LanguageCode;
formTranslations: NewsletterPopupTranslations;
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
type NewsletterForm = { | |
title: string; | |
lang: LanguageCode; | |
formTranslations: NewsletterPopupTranslations; | |
}; | |
type NewsletterGlowContainerProps = { | |
title: string; | |
lang: LanguageCode; | |
formTranslations: NewsletterPopupTranslations; | |
}; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: Shouldn't redeclare 'NewsletterForm'. Consider to delete it or rename it.
'NewsletterForm' is defined here:
(lint/suspicious/noRedeclare)
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/src/components/newsletter-glow-container/newsletter-glow-container.tsx (1)
18-23
: Simplify nested div structure.The nested divs with flex and alignment can be consolidated.
- <div className="flex flex-col items-center py-12"> - <div className="align-center flex flex-col"> - <Typography size="2xl" color="foreground" weight="medium"> - {title} - </Typography> - </div> + <div className="flex flex-col items-center py-12"> + <Typography size="2xl" color="foreground" weight="medium" className="text-center"> + {title} + </Typography>website/src/components/newsletter-form/newsletter-form.tsx (1)
35-37
: Use optional chaining.Replace nested condition with optional chaining operator.
- if (t && t.id) { - toast.dismiss(t.id); - } + t?.id && toast.dismiss(t.id);🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/src/components/newsletter-form/newsletter-form.tsx
(1 hunks)website/src/components/newsletter-glow-container/newsletter-glow-container.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
website/src/components/newsletter-form/newsletter-form.tsx
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (2)
website/src/components/newsletter-form/newsletter-form.tsx (2)
30-32
:⚠️ Potential issueSupport all available language codes.
The language selection is overly simplified and doesn't support all available languages.
- language: lang === 'de' ? 'de' : 'en', + language: lang,Likely invalid or redundant comment.
33-43
: 🛠️ Refactor suggestionImprove error handling with async/await.
Convert Promise chain to async/await for better error handling.
- api.post('/api/newsletter/subscription/public', body).then((response) => { - if (response.status === 200) { - if (t && t.id) { - toast.dismiss(t.id); - } - toast.success(translations.toastSuccess); - } else { - toast.error(translations.toastFailure); - } - }); + try { + const response = await api.post('/api/newsletter/subscription/public', body); + if (response.status === 200) { + t?.id && toast.dismiss(t.id); + toast.success(translations.toastSuccess); + } else { + toast.error(translations.toastFailure); + } + } catch (error) { + toast.error(translations.toastFailure); + }Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
UPD: I've tested the newsletter form on staging just to be sure that nothing changed and everything works as expected. |
This PR introduces a new component,
GlowHoverContainer
, which extendsBaseContainer
with a glowing cursor effect.Issue: #987
Changes:
GlowHoverContainer
: a new component that wrapsBaseContainer
and adds the glowing cursor effect.BaseContainer
: minor adjustments to enable the glowing cursor functionality:backgroundColor
property, which was previously used on both the parent and child<div>
s within the component. This change appears to have no impact on existing functionality, as I've found only 1 usage ofbackgroundColor
property in the project (on the /arts page) and it works fine after component's change.NewsletterGlowContainer
as a new component, which containsNewsletterForm
(also used on this page in the toast) to the Campaign pageNote:
npm run format-code
produced unexpected changes across all repo files, primarily related to line breaks. To avoid unintended consequences, I did not commit these changes.I understand that consistent code formatting is important. However, I'm encountering difficulties with the current formatting command locally. If you have any suggestions or have ever encountered the same issue, I'd really appreciate any advice.
Hope this helps!
Summary by CodeRabbit
New Features
Improvements
Technical Updates