feat(web): Add default header for fiskistofa organization #15973
Conversation
WalkthroughThe changes in this pull request introduce a new conditional rendering mechanism for the organization header in the Changes
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
Documentation and Community
|
![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 (1)
apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/FiskistofaDefaultHeader.tsx (1)
1-65: TheFiskistofaDefaultHeadercomponent implementation looks good! 👍The component follows the NextJS best practices and efficiently manages the state using custom hooks. It is designed to be rendered on the server-side and uses TypeScript effectively to ensure type safety. The component is modular, reusable, and promotes code reuse by using the
DefaultHeadercomponent from the shared library. The use of a custom CSS module helps to avoid CSS conflicts and keeps the styles scoped to the component.A few additional suggestions to further improve the component:
- Consider extracting the background style logic into a separate function or variable to improve readability and maintainability.
- Add PropTypes or default props to the component to improve documentation and provide better developer experience.
- Consider adding a loading state or placeholder for the logo image to improve the user experience while the image is loading.
+ const getBackgroundStyle = (width: number, isSubpage: boolean, themeProp: OrganizationPage['themeProperties']) => { + if (width > theme.breakpoints.lg && !isSubpage) { + return themeProp.backgroundColor + } + return 'no-repeat 52% 30% ,linear-gradient(180deg, #E6F2FB 21.56%, #90D9E3 239.74%)' + } + + FiskistofaDefaultHeader.propTypes = { + organizationPage: PropTypes.object.isRequired, + logoAltText: PropTypes.string.isRequired, + isSubpage: PropTypes.bool.isRequired, + } + + FiskistofaDefaultHeader.defaultProps = { + isSubpage: false, + }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- apps/web/components/Organization/Wrapper/OrganizationWrapper.tsx (2 hunks)
- apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/FiskistofaDefaultHeader.css.ts (1 hunks)
- apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/FiskistofaDefaultHeader.tsx (1 hunks)
- apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/index.ts (1 hunks)
Additional context used
Path-based instructions (4)
apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/FiskistofaDefaultHeader.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/FiskistofaTheme/index.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/FiskistofaTheme/FiskistofaDefaultHeader.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."
apps/web/components/Organization/Wrapper/OrganizationWrapper.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 not posted (6)
apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/FiskistofaDefaultHeader.css.ts (1)
1-6: LGTM!The CSS module follows best practices and adheres to the project's conventions:
- The exported constant uses camelCase, which is the recommended naming convention for CSS Modules.
- The
@vanilla-extract/csslibrary is utilized, providing type safety and other benefits associated with CSS-in-JS solutions.- The maximum width of 1342px seems appropriate for a header component, assuming it aligns with the design requirements.
- Centering the element horizontally using
margin: '0 auto'is a common and effective technique.apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/index.ts (2)
3-3: LGTM!The import statement is correct and necessary for exporting
FiskistofaDefaultHeaderlater in the file.
11-11: LGTM!The export statement is correct and aligns with the PR objective of introducing a default header for the Fiskistofa organization. It enhances the module's functionality by providing an additional header component that can be utilized in other parts of the application.
apps/web/components/Organization/Wrapper/OrganizationWrapper.tsx (3)
62-65: LGTM!The import statements are correct and necessary for the conditional rendering logic introduced later in the file.
334-340: Excellent work!The conditional rendering logic for the
fiskistofaorganization header is implemented correctly. TheFiskistofaDefaultHeadercomponent receives the necessary props to render correctly based on the conditionn('usingDefaultHeader', false). The changes adhere to the best practices and are consistent with the existing code.
335-339: Great job!The
isSubpageprop is correctly passed to theFiskistofaDefaultHeadercomponent based on the condition(isSubpage && n('smallerSubpageHeader', false)) ?? false. This ensures that theisSubpageprop is set tofalseif eitherisSubpageisfalseorn('smallerSubpageHeader', false)evaluates tofalse. The changes are consistent with the existing code and follow the best practices.
apps/web/components/Organization/Wrapper/Themes/FiskistofaTheme/FiskistofaDefaultHeader.tsx
Show resolved
Hide resolved
Datadog ReportBranch report: ✅ 0 Failed, 84 Passed, 0 Skipped, 29.87s Total Time |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15973 +/- ##
==========================================
+ Coverage 36.85% 36.86% +0.01%
==========================================
Files 6715 6716 +1
Lines 137690 137584 -106
Branches 39143 39105 -38
==========================================
- Hits 50744 50721 -23
+ Misses 86946 86863 -83
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Add default header for fiskistofa organization
What
Make it possible to use default header for syslumenn organization via config.
Why
A design that was approved by Digital Iceland
Screenshots / Gifs
Before
After
Checklist:
Summary by CodeRabbit
FiskistofaDefaultHeadercomponent that adapts to different contexts and screen sizes.FiskistofaDefaultHeadercomponent for enhanced module functionality.