Website: Survey page (fixing bugs) #971
Conversation
- Cookie banner - Region selection dropdown
dnhn
requested review from
andrashee,
mkue,
ssandino and
socialincome-dev
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@dnhn is attempting to deploy a commit to the Social Income Team on Vercel. A member of the Team first needs to authorize it. |
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request include modifications to a CSS file, the introduction of a new custom React hook, and updates to two components to utilize this hook. The CSS variable Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
website/src/components/tracking/cookie-consent-banner.tsx (1)
17-23: Add type annotation and document the survey page exceptionThe survey page exclusion logic is correct but could be more explicit.
Consider these improvements:
- const isSurveyPage = useIsPage('survey'); + const isSurveyPage: boolean = useIsPage('survey'); useEffect(() => { const cookieConsent = localStorage.getItem('cookie_consent'); + // Hide banner for survey pages or when consent is already given setHideBanner(Boolean(cookieConsent) || isSurveyPage); }, [isSurveyPage, setHideBanner]);website/src/components/i18n-dialog.tsx (1)
52-52: Improve code organization and documentationThe survey page check should be closer to where it's used, and the special case should be documented.
Consider moving and documenting the check:
- const isSurveyPage = useIsPage('survey'); const [open, setOpen] = useState(false); const { language, setLanguage, region, setRegion, currency, setCurrency } = useI18n(); // ... other code ... + // Hide region selection for survey pages as they're region-independent + const isSurveyPage = useIsPage('survey'); {!_.isEmpty(regions) && !isSurveyPage && (Also applies to: 79-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
website/src/app/[lang]/[region]/survey/[recipient]/[survey]/survey.css(1 hunks)website/src/components/i18n-dialog.tsx(3 hunks)website/src/components/tracking/cookie-consent-banner.tsx(2 hunks)website/src/hooks/useIsPage.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- website/src/app/[lang]/[region]/survey/[recipient]/[survey]/survey.css
|
Testing on the staging user nDJwnvktHXolcAiYYnAm and his Background gone: (left = new) Removes region selection dropdown: (left = new) Also Cookie banner is not appearing. Great job @dnhn ! Next will be a technical review by @mkue |
| @@ -0,0 +1,12 @@ | |||
| import { usePathname } from 'next/navigation'; | |||
|
|
|||
| export function useIsPage(page: string) { | |||
There was a problem hiding this comment.
This hook seems to have a quite specific use-case. It only checks for the third item in the path segments. Maybe a hook usePathnameContains(page: string) would be more re-usable?
You then still do
const isSurveyPage = usePathnameContains('/[lang]/[region]/survey');What do you think?
There was a problem hiding this comment.
That’s a good idea, I’ll update it.
Hi team,
I found this project through For Good First Issue. After reviewing issues #759, #760, #761, I noticed they are all related to the Survey page and have remained unresolved since their last activities. I decided to investigate and fix them.
Let me know if you have any feedback!
Summary by CodeRabbit
Release Notes
New Features
useIsPageto determine if the current page is a survey page, enhancing component logic.I18nDialogcomponent based on the current page type.CookieConsentBannerto hide the banner on survey pages.Style
--background-dimCSS variable totransparentfor improved visual clarity on survey pages.