-
Notifications
You must be signed in to change notification settings - Fork 2
Feat/443 ux UI improvements incident map #444
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe updates focus on enhancing the UI and user experience for the incident map and related components. Changes include improved sidebar organization, new shadow and z-index styles, a prominent mobile call-to-action button, updated translation keys, and expanded Tailwind and CSS utilities for consistent styling across mobile and desktop interfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MobileSidebar
participant ButtonLink
participant RootPage
User->>MobileSidebar: Opens sidebar on mobile
MobileSidebar->>User: Displays sidebar with new layout and call-to-action
User->>ButtonLink: Taps "Report now" button
ButtonLink->>RootPage: Opens root page in new tab
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🪛 Biome (1.9.4)src/app/[locale]/incident-map/components/IncidentMapContent.tsx[error] 24-24: Do not shadow the global "Map" property. Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global. (lint/suspicious/noShadowRestrictedNames) ⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (2)
src/app/[locale]/incident-map/components/IncidentMapHeader.tsx (1)
92-99
: Desktop call-to-action button added.The conditional rendering of the "create incident" ButtonLink for non-mobile views is well-implemented. This provides a consistent user experience with the mobile UI while adapting appropriately to the desktop layout.
One suggestion regarding the target URL:
Consider using a more specific route instead of the root path ("/") to take users directly to the incident creation form, rather than just the homepage. This would provide a more direct user flow.
src/app/[locale]/incident-map/components/IncidentMapContent.tsx (1)
24-24
: Consider renaming the Map component import to avoid shadowingThe Map component is shadowing the global Map property.
-import { Map } from '@/components/ui/Map' +import { Map as MapComponent } from '@/components/ui/Map'And then update its usage:
- <Map + <MapComponent🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/app/[locale]/incident-map/components/IncidentMapContent.tsx
(4 hunks)src/app/[locale]/incident-map/components/IncidentMapHeader.tsx
(2 hunks)src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx
(4 hunks)src/app/[locale]/incident-map/components/NestedCategoryCheckboxList.tsx
(4 hunks)src/app/[locale]/incident/add/components/MapDialog.css
(1 hunks)src/app/[locale]/incident/add/components/MapDialogContent.tsx
(1 hunks)src/app/[locale]/incident/add/components/MapDialogMobileContent.tsx
(2 hunks)src/app/nl-design-system-overrides.css
(1 hunks)tailwind.config.ts
(1 hunks)translations/en.json
(1 hunks)translations/nl.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app/[locale]/incident-map/components/NestedCategoryCheckboxList.tsx (1)
src/types/category.ts (1)
ParentCategory
(36-41)
src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx (1)
src/components/index.ts (1)
SpotlightSection
(38-38)
🪛 Biome (1.9.4)
src/app/[locale]/incident-map/components/IncidentMapContent.tsx
[error] 24-24: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (26)
translations/en.json (1)
209-210
: Good typo fix and useful addition for the mobile CTA.Fixed the typo in
category_icon_alt_text
("fot" → "for") which improves accessibility for screen readers. The newcreate_incident
translation key "Report now" supports the call-to-action button in the incident map UI mentioned in the PR objectives.tailwind.config.ts (1)
14-24
: Nice enhancement of design tokens for consistent styling.The addition of box shadow utilities (
shadow-right
,shadow-bottom
,shadow-top
) and color utilities (text-primary
,bg-primary
) provides consistent styling options that support the UI improvements mentioned in the PR objectives. The shadows will help with layering and visual hierarchy in the incident map components.src/app/[locale]/incident-map/components/NestedCategoryCheckboxList.tsx (4)
6-6
: LGTM: Adding clsx for conditional class handling.Good addition of the clsx utility to handle conditional class name composition.
12-13
: LGTM: Mobile responsiveness improvement.The addition of the optional
mobile
prop with a default value offalse
allows the component to adapt its styling based on the device context, supporting the UI improvements for mobile mentioned in the PR objectives.Also applies to: 32-33
132-137
: LGTM: Mobile-friendly text sizing for parent categories.The conditional application of the
text-lg
class for parent category labels when in mobile view improves readability and touch target sizes on smaller screens.
168-172
: LGTM: Consistent mobile text sizing for subcategories.Similar to the parent categories, the subcategories now also receive the
text-lg
class in mobile view, ensuring consistency in the UI.src/app/[locale]/incident/add/components/MapDialog.css (1)
69-69
: Repositioned zoom controls to avoid overlap with new UI elements.Increasing the bottom position of the zoom button group from what was likely 2.4em to 6.4em in mobile view ensures that these controls don't overlap with the new call-to-action button or other UI elements introduced in this PR.
translations/nl.json (1)
208-209
: LGTM: Translation key added successfully.The new translation key "create_incident" with the Dutch text "Doe een melding" is appropriately positioned and will support the new call-to-action buttons in the incident map UI.
src/app/[locale]/incident/add/components/MapDialogContent.tsx (1)
96-96
: Visual enhancement looks good.The addition of
shadow-right
andz-10
classes improves the visual hierarchy by adding a right-side shadow and setting an appropriate z-index for the sidebar container. This enhances the separation between UI elements and aligns with the overall UI improvements in this PR.src/app/[locale]/incident/add/components/MapDialogMobileContent.tsx (2)
103-107
: Conditional shadow styling enhances mobile UX.The dynamic styling with
z-10
class and conditionalshadow-bottom
provides appropriate visual cues based on the UI state. This enhances the mobile experience by applying shadows only when needed.
190-191
: Consistent shadow styling for map container.The addition of
z-10
class and conditionalshadow-top
creates a consistent visual layer pattern with the header container. This approach maintains visual hierarchy between UI elements in different states.src/app/nl-design-system-overrides.css (1)
177-185
: Mobile button styling supports new UI components.The new CSS rules for
.utrecht-button-link.mobile
and.utrecht-button-link.mobile.full-width
provide appropriate styling for the new call-to-action buttons in the incident map UI. The larger text size and full-width options enhance visibility and usability on mobile devices.src/app/[locale]/incident-map/components/IncidentMapHeader.tsx (1)
10-23
: Import statements updated correctly.The imports have been expanded to include necessary components and utilities for the new button functionality. The explicit import of React alongside hooks follows best practices.
src/app/[locale]/incident-map/components/IncidentMapContent.tsx (7)
21-21
: The new imports support the UI enhancementsAll imported components are appropriately used in the UI improvements, with SpotlightSection adding semantic emphasis to descriptions, IconArrowRight enhancing the new CTA button, and ButtonLink providing the mobile action component.
Also applies to: 25-30, 48-48
309-309
: Good visual layering with shadow and z-indexAdding
shadow-right
andz-10
classes creates a clear visual hierarchy and improves the separation between the sidebar and map content.
311-317
: Improved spacing around incident detailsThe padding around SelectedIncidentDetails improves readability and visual organization of the content.
320-322
: Enhanced emphasis with SpotlightSectionUsing SpotlightSection for the incident map description provides better semantic structure and visual emphasis for this important information.
323-334
: Better organization of address input and filtersThe reorganized structure with proper padding creates a more cohesive grouping of related elements and improves the overall layout.
342-342
: Consistent shadow styling for mobile containerThe
shadow-bottom
andz-10
classes maintain the visual hierarchy pattern established in the non-mobile view.
466-475
:✅ Verification successful
Prominent mobile CTA improves accessibility
The fixed bottom bar with full-width ButtonLink creates a clear and accessible action point for mobile users. This aligns with the design improvements mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify that mobile styling classes exist in CSS rg -A 2 "\.mobile\.full-width" --glob "*.css"Length of output: 254
Mobile CTA styling verified and approved
Confirmed that
.utrecht-button-link.mobile.full-width
is defined insrc/app/nl-design-system-overrides.css
, ensuring the full-width ButtonLink displays correctly on mobile. This implementation aligns with the PR objectives and provides a clear, accessible action point.src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx (6)
12-12
: Appropriate import for enhanced UI elementThe SpotlightSection import supports the improved semantic structure for description content.
63-63
: Improved mobile drawer trigger positioning and stylingRepositioning the trigger button (
bottom-16
instead of bottom-0) helps prevent overlap with other UI elements, while the reduced height and shadow-top style enhance the visual appearance.
68-68
: Enhanced drawer content with proper layeringThe addition of
shadow-top
andz-20
classes creates a clear visual hierarchy and improves the distinction between overlapping elements.
85-91
: Better semantic structure for description contentWrapping the description in SpotlightSection improves the visual emphasis and semantic meaning, maintaining consistency with the non-mobile implementation.
93-118
: Well-structured content with appropriate spacingThe restructured drawer content with proper padding and font sizing creates a more organized and readable layout. The addition of the
mobile={true}
prop to NestedCategoryCheckboxList enables mobile-specific styling, enhancing the responsive design.
132-132
: Improved drag handle for better touch interactionThe wider, shorter drag handle with primary color styling improves visibility and touch interaction on mobile devices.
This comment was marked as resolved.
This comment was marked as resolved.
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/[locale]/incident-map/components/IncidentMapContent.tsx
(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/[locale]/incident-map/components/IncidentMapContent.tsx
[error] 24-24: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e
🔇 Additional comments (7)
src/app/[locale]/incident-map/components/IncidentMapContent.tsx (7)
21-21
: LGTM: Import additions support the UI improvements.The new imports for
SpotlightSection
,IconArrowRight
, andButtonLink
are properly added and align with the UI enhancement objectives.Also applies to: 25-30, 48-48
289-289
: Verify the layout changes address the reported desktop overflow issue.The container layout has been updated with new grid configuration and height constraints. This should help with the reported issue of filters expanding outside the container on desktop.
The user feedback mentioned that "on desktop, the filters expand outside the container, causing the page to scroll unnecessarily." Please verify that the new
overflow-y-hidden
and height constraints (min-h-[calc(100vh-102px)]
) effectively address this issue across different desktop screen sizes.
309-309
: Good use of shadow and z-index for visual layering.The addition of
shadow-right z-10
andoverflow-y-auto
with max height constraint improves the visual hierarchy and prevents content overflow in the sidebar.
311-317
: Improved content organization with proper padding.Wrapping the
SelectedIncidentDetails
component in a padded container provides better visual spacing and content organization.
320-322
: Enhanced semantic markup with SpotlightSection.Replacing the plain paragraph with
SpotlightSection
component provides better semantic meaning and consistent styling. This may also help address the dark mode readability issue mentioned in user feedback.The user feedback mentioned "in dark mode, the informational text is not readable." Please verify that the
SpotlightSection
component withtype="info"
provides adequate contrast and readability in dark mode.
323-335
: Better content grouping with improved layout structure.The address combobox and category checkbox list are now properly grouped within a padded container, which enhances the visual organization of the sidebar content.
342-342
: Consistent mobile styling with shadow effects.The mobile address input container now uses consistent shadow and z-index styling that matches the desktop sidebar approach.
{isMobile && ( | ||
<div className="flex items-center justify-center bg-white py-3 z-30 absolute bottom-0 w-full p-4"> | ||
<ButtonLink href="/" target="_blank" className="mobile full-width"> | ||
{tIncidentMap('create_incident')} | ||
<Icon> | ||
<IconArrowRight /> | ||
</Icon> | ||
</ButtonLink> | ||
</div> | ||
)} |
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.
💡 Verification agent
🧩 Analysis chain
Verify the mobile call-to-action positioning addresses the filter accessibility issue.
The new fixed bottom bar with the "create_incident" button provides a prominent call-to-action on mobile. However, this needs verification against the reported issue.
The user feedback stated that "on mobile devices, the filters appear below the 'Doe een melding' (Make a report) button, which prevents users from toggling the filters effectively." Please verify that:
- This fixed bottom button doesn't interfere with filter interactions
- The filters remain accessible above this button
- The button positioning doesn't prevent users from scrolling to see all filter options
🏁 Script executed:
#!/bin/bash
# Check if there are mobile-specific filter components that could be affected
rg -A 10 -B 5 "IncidentMapMobileSidebar|NestedCategoryCheckboxList.*mobile" --type tsx
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Search for mobile-specific filter components and their code context
rg -A 10 -B 5 'IncidentMapMobileSidebar|NestedCategoryCheckboxList'
# Search for any filter-related TSX files to see how filters are rendered
rg -i 'filter' -A 5 -B 5 -g '*.tsx'
Length of output: 39414
Fix mobile CTA overlapping the filter toggle
The bottom “create_incident” button is fixed with z-30
in IncidentMapContent.tsx, but the filter toggle (Drawer.Trigger
) in IncidentMapMobileSidebar.tsx has no z-index, causing it to render beneath the CTA and blocking users from opening the filters.
Please update the z-index on the filter toggle so it sits above the CTA, for example:
• File: src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx
<Drawer.Trigger
- className="absolute bottom-16 w-full flex h-10 flex-shrink-0 items-center justify-center overflow-hidden bg-white px-4 text-base font-medium transition-all hover:bg-[#FAFAFA] dark:bg-[#161615] dark:hover:bg-[#1A1A19] shadow-top"
+ className="absolute bottom-16 w-full flex h-10 flex-shrink-0 items-center justify-center overflow-hidden bg-white px-4 text-base font-medium transition-all hover:bg-[#FAFAFA] dark:bg-[#161615] dark:hover:bg-[#1A1A19] shadow-top z-40"
>
Verify after this change that:
- The filter drawer opens when tapping the filter toggle.
- The drawer content remains fully scrollable (up to its snap height).
- The CTA no longer covers any part of the filter UI.
📝 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.
{isMobile && ( | |
<div className="flex items-center justify-center bg-white py-3 z-30 absolute bottom-0 w-full p-4"> | |
<ButtonLink href="/" target="_blank" className="mobile full-width"> | |
{tIncidentMap('create_incident')} | |
<Icon> | |
<IconArrowRight /> | |
</Icon> | |
</ButtonLink> | |
</div> | |
)} | |
--- a/src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx | |
+++ b/src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx | |
@@ -1,7 +1,7 @@ | |
<Drawer.Trigger | |
- className="absolute bottom-16 w-full flex h-10 flex-shrink-0 items-center justify-center overflow-hidden bg-white px-4 text-base font-medium transition-all hover:bg-[#FAFAFA] dark:bg-[#161615] dark:hover:bg-[#1A1A19] shadow-top" | |
+ className="absolute bottom-16 w-full flex h-10 flex-shrink-0 items-center justify-center overflow-hidden bg-white px-4 text-base font-medium transition-all hover:bg-[#FAFAFA] dark:bg-[#161615] dark:hover:bg-[#1A1A19] shadow-top z-40" | |
> | |
{/* …filter toggle contents… */} | |
</Drawer.Trigger> |
🤖 Prompt for AI Agents
In src/app/[locale]/incident-map/components/IncidentMapMobileSidebar.tsx around
the lines where the filter toggle (Drawer.Trigger) is implemented, increase its
z-index to a value higher than 30 so it renders above the fixed bottom
"create_incident" button with z-30 in IncidentMapContent.tsx. This will prevent
the CTA button from overlapping and blocking the filter toggle. After updating
the z-index, test that tapping the filter toggle opens the drawer, the drawer
content is fully scrollable, and the CTA button no longer covers any filter UI
elements.
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.
Wat mij betreft prima om te mergen!
Zie ontwerp Jake: https://www.figma.com/design/AWXlC2SNvqGuEOYxGqZJc1/Signalen?node-id=34-2&t=Rt0q3cPK5cfuxV9v-1
Summary by CodeRabbit
New Features
Style
Bug Fixes
Documentation