fix(web): Change custom syslumenn pages config for header #16299
Conversation
WalkthroughThe pull request introduces changes to three components: 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
|
![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 (3)
apps/web/screens/Organization/Syslumenn/Homestay.tsx (2)
375-376: LGTM! Consider adding type assertion for improved type safety.The introduction of the
usingDefaultHeadervariable is a good addition that aligns with the PR objective. The use of the nullish coalescing operator for providing a default value is a good practice.For improved type safety, consider adding a type assertion to the
namespaceobject:const usingDefaultHeader: boolean = (namespace as { usingDefaultHeader?: boolean })['usingDefaultHeader'] ?? false;This ensures that TypeScript understands the structure of the
namespaceobject and prevents potential type-related issues.
383-388: LGTM! Consider improving object literal formatting for better readability.The conditional setting of
themeConfigbased onusingDefaultHeaderis a good implementation that aligns with the PR objective. It ensures that the header color scheme is correctly configured based on whether the default header is being used or not.Consider improving the formatting of the object literal for better readability:
themeConfig: !usingDefaultHeader ? { headerButtonColorScheme: 'negative', headerColorScheme: 'white', } : {},This format aligns the object properties and makes the code structure more clear.
apps/web/screens/Organization/Syslumenn/OperatingLicenses.tsx (1)
671-683: LGTM! Consider minor readability improvement.The changes look good and align with the PR objectives. The introduction of
usingDefaultHeaderadds flexibility to the header configuration, and the conditional setting ofthemeConfigallows for dynamic theming.A minor suggestion for improved readability:
- const usingDefaultHeader: boolean = namespace['usingDefaultHeader'] ?? false + const usingDefaultHeader = namespace['usingDefaultHeader'] ?? falseThe explicit
: booleantype annotation is unnecessary here as TypeScript can infer the type from the right-hand side of the assignment.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- apps/web/screens/Organization/Syslumenn/Auctions.tsx (1 hunks)
- apps/web/screens/Organization/Syslumenn/Homestay.tsx (1 hunks)
- apps/web/screens/Organization/Syslumenn/OperatingLicenses.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/web/screens/Organization/Syslumenn/Auctions.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/screens/Organization/Syslumenn/Homestay.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/screens/Organization/Syslumenn/OperatingLicenses.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/screens/Organization/Syslumenn/Homestay.tsx (1)
375-388: Overall changes align well with PR objectives and follow best practices.The modifications to the
Homestaycomponent, particularly in thegetPropsmethod, effectively implement the desired functionality for custom syslumenn pages configuration. The introduction of theusingDefaultHeadervariable and the conditional setting ofthemeConfigensure that the header color scheme aligns correctly with the default header configuration on the island.is platform.The changes adhere to TypeScript and NextJS best practices, including efficient state management and type safety. The code is concise, readable, and maintains the overall structure and functionality of the component.
Great job on implementing these changes! They successfully address the PR objectives while maintaining code quality and following best practices.
apps/web/screens/Organization/Syslumenn/Auctions.tsx (3)
Line range hint
1-1102: LGTM: Well-structured React component with robust filtering logic.The Auctions component is well-implemented, following React and TypeScript best practices. It effectively uses island.is UI components and custom hooks, handling complex filtering and display logic for auctions.
1103-1116: LGTM: Dynamic header configuration aligns with PR objectives.The introduction of
usingDefaultHeaderand the conditionalthemeConfigeffectively implements the required changes for custom syslumenn pages. This change allows for dynamic header styling, which aligns well with the PR's objective of ensuring correct header color scheme alignment.
1117-1122: LGTM: Proper component export with layout wrapper.The export statement correctly uses the
withMainLayouthigher-order component, following good practices for component organization and layout management.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16299 +/- ##
==========================================
- Coverage 36.86% 36.83% -0.03%
==========================================
Files 6798 6796 -2
Lines 140486 140412 -74
Branches 39936 39931 -5
==========================================
- Hits 51792 51726 -66
+ Misses 88694 88686 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
…-pages (#16234) * Service portal removal. Add portals my pages * minor fixes * Fix * path fix * fix(portals-admin): locklist (#16279) * fix(portals-admin): locklist * tweak * msg id fix * tweak --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): feature flag resolver for documents (#16285) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(vehicles-bulk-mileage): Fixes after testing review (#16295) * fix: testing fixes v1 * fix: testing comments v2 * fix: better message * fix: function name * fix: duplicate loading --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(tests): New @island/testing/e2e library (#16287) * Add @swc-node/register and @swc/core * Add testing/e2e library * update project.json for testing/e2e * fix import for libTestingE2e --------- Co-authored-by: Kristofer <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(parental-leave): ApplicationRights (#15901) * feat(parental-leave): ApplicationRights Added applicationRights to parental-leave when sending application. Since we are using a new way of calculating periods * Fix days used by period calculation * Tests for new periods * rename function with proper camelCase * Refactor: Made duplicate code into a function * Make ApplicationRights nullable * refactor: function instead of duplicate code * remove console.log * error handling for period data * clientConfig nullable fix * Fixes for calculation of months. And using clamp to get correct value of daysLeft * Multiply amount of months by 30 for period calculation with month durations * Fix old calculation of endDate with months --------- Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(passport-application): Updated readme (#16296) * updated readme * updated readme * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): date format signature, remove self affect, disclaimer text (#16288) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): No diff no addition in appendix (#16293) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Global alert banner - Handle null case (#16298) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Change custom syslumenn pages config for header (#16299) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(j-s): Digital mailbox API (#16301) * feat(j-s): Block create subpoena on staging and dev * Update subpoena.service.ts * fix(j-s): Fix mailbox API * remove changes not meant for this branch * Update subpoena.service.ts * fix(j-s): reverting changes from other branch * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): Fix list reviewed toggle (#16300) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(scripts): Stricter shell script checking (#16242) * Set style level for shellcheck * Linting & formatting scripts * Remove _podman.sh script * Format all scripts * Add reviewdog/action-shfmt step * Configure shfmt * Merge from main * Linting * Move shfmt to before lint * Remove reviewdog * Allow external sources in shellcheck * Use Reviewdog for shellcheck * Set version for Reviewdog --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(new-primary-school): Update messages namespace (#16302) Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(driving-license): check if 65+ renewal is possible (#16292) * check if 65 renewal is possible * remove console log * cleanup * coderabbit tweaks * coderabbit changes * quick fix * add type? --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): default defender and has chosen fields for subpoena (#16306) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function * feat: add default choices ans has chosen + loading states * fix: use type * fix: undefined type issue * fix: simplify check * Update service setup for my pages infra * chore: charts update dirty files * Remove from infra * undo rename --------- Co-authored-by: albinagu <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> Co-authored-by: Þorkell Máni Þorkelsson <[email protected]> Co-authored-by: Svanhildur Einarsdóttir <[email protected]> Co-authored-by: Kristofer <[email protected]> Co-authored-by: helgifr <[email protected]> Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: Rafn Árnason <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: Rúnar Vestmann <[email protected]> Co-authored-by: mannipje <[email protected]> Co-authored-by: unakb <[email protected]> Co-authored-by: juni-haukur <[email protected]> Co-authored-by: birkirkristmunds <[email protected]> Co-authored-by: Kristján Albert <[email protected]>
Change custom syslumenn pages config for header
What
Change config setup for custom syslumenn pages.
Why
So the island.is header color scheme is right when using default header config.
Screenshots / Gifs
Before
After
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes