feat(web): Sýslumenn footer changes #16945
Conversation
WalkthroughThe pull request introduces updates to the Changes
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16945 +/- ##
==========================================
+ Coverage 36.41% 36.43% +0.01%
==========================================
Files 6898 6891 -7
Lines 144874 144728 -146
Branches 41404 41368 -36
==========================================
- Hits 52755 52730 -25
+ Misses 92119 91998 -121
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 15 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Affected services are: web, Deployed services: web. |
Datadog ReportBranch report: ✅ 0 Failed, 84 Passed, 0 Skipped, 36.06s Total Time |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts (1)
17-19: Add documentation for the new style.The implementation looks good and follows the established patterns. Consider adding a comment to document the purpose and usage of this style.
+// Background style for service web footer variant export const serviceWebFooterBg = style({ background: theme.color.purple100, })apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx (3)
67-67: Consider extracting the color type for reusabilityThe color prop updates are consistent and type-safe. However, since
keyof typeof theme.coloris used multiple times, consider extracting it into a reusable type.type ThemeColor = keyof typeof theme.color;Also applies to: 77-77, 112-112
139-143: Simplify the color conditionThe ternary operation for link color can be simplified.
-color={footerTextColor === 'white' ? 'white' : 'blue400'} +color={footerTextColor === 'dark400' ? 'blue400' : 'white'}
140-143: Consider making the privacy policy URL configurableThe privacy policy URL is hardcoded in the localization key. Consider moving it to a configuration file for better maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts(2 hunks)apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx(11 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (4)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts (1)
17-19: Verify color contrast accessibility.
Please ensure that the purple100 background color provides sufficient contrast with the text colors used in the footer for WCAG compliance.
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx (3)
17-17: LGTM! Clean variable declarations with good type safety
The new theme import and conditional variables enhance type safety and maintainability while following TypeScript best practices.
Also applies to: 47-50
179-179: LGTM! Well-typed interface extension
The HeaderLink interface update maintains type safety while providing good flexibility with the optional color prop.
Also applies to: 187-187
Line range hint 1-224: Verify client-side only code protection
While the component follows NextJS best practices, let's verify there's no client-side only code that needs protection.
RunarVestmann
added
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts (1)
17-19: Consider adding a descriptive commentThe style implementation is clean and correctly uses the theme system. Consider adding a brief comment explaining when this background variant should be used.
+// Background style for service web variant of the footer export const serviceWebFooterBg = style({ background: theme.color.purple100, })apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx (1)
137-163: Consider extracting privacy policy link into a separate componentThe privacy policy link section contains complex conditional rendering and styling logic. Consider extracting it into a dedicated component to improve maintainability and readability.
Example refactor:
interface PrivacyPolicyLinkProps { footerTextColor: keyof typeof theme.color; n: (key: string, defaultValue: string | boolean) => string; } const PrivacyPolicyLink: FC<PrivacyPolicyLinkProps> = ({ footerTextColor, n }) => { if (!n('showPrivacyPolicyLink', true)) return null; return ( <Box marginTop={3} className={styles.linkMaxWidth}> <Link color={footerTextColor === 'white' ? 'white' : 'blue400'} href={n('privacyPolicyHref', '/s/syslumenn/personuverndarstefna-syslumanna')} skipTab > <Button size="small" colorScheme="negative" icon="arrowForward" variant="text" as="span" > {n('privacyPolicyLabel', 'Persónuverndarstefna Sýslumanna')} </Button> </Link> </Box> ); };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts(2 hunks)apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx(10 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (6)
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.css.ts (2)
1-3: LGTM! Clean import structure
The import statements follow TypeScript best practices and properly utilize the organization's UI theme package.
21-27: Verify the fixed maxWidth value
The responsive style implementation is correct, but please verify if the fixed 200px maxWidth for large screens is the intended design value.
apps/web/components/Organization/Wrapper/Themes/SyslumennTheme/SyslumennFooter.tsx (4)
17-17: LGTM! Type-safe theme implementation
The theme import and conditional variables are well-implemented with proper TypeScript type safety through the use of keyof typeof theme.color.
Also applies to: 47-50
67-67: LGTM! Consistent color implementation
The dynamic text color is consistently applied across all Text components, maintaining visual coherence based on the isServiceWeb context.
Also applies to: 77-77, 112-113
124-124: LGTM! Well-structured component enhancement
The HeaderLink component updates properly handle dynamic styling while maintaining type safety and localization support.
Also applies to: 135-135
183-183: LGTM! Well-typed interface extension
The HeaderLink interface is properly extended with the optional color prop, maintaining type safety with theme colors.
Also applies to: 191-191
* Syslumenn footer changes * Add max width to link * Potentially hide privacy policy link --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sýslumenn footer changes
Digital Iceland requested changes to "Sýslumenn" footer on the web.
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes