feat: initial sidebar implementation #11

georgi4444 · 2024-11-04T14:08:55Z

used nextui card instead of shadcn implementation
added context provider with the state of the sidebar and information about clicked map type and alert type
can be closed and opened
alert menu bottom right when sidebar is closed
dark/light mode switch

todos:

missing alert info when hovered
missing input field when sidebar is closed
maptypes shouldn't lose selected styling when out of focus

Tschonti

generally, all types and interfaces should be defined in the domain folder

Tschonti · 2024-11-04T14:37:48Z

src/components/AlertsMenu/AlertsMenu.tsx

+              <Button isIconOnly radius="full" className={isSubAlertClicked(item.key) ? 'bg-primary' : 'bg-background'}>
+                <div className="w-6 h-6 flex items-center justify-center">
+                  <Image
+                    src={item.icon}
+                    alt={item.label}
+                    className="object-contain w-auto h-auto max-w-full max-h-full"
+                  />
+                </div>
+              </Button>


it seems like this is repeated 3 times in this file. Can't we create a component from this? Olcick and className could be set with props

I've tried but couldn't make the popover trigger work when I provide it with my custom button function component

You should provide a function in the custom component props

Solved: had to forward the ref

Tschonti · 2024-11-04T14:38:26Z

src/components/AlertsMenu/AlertsMenu.tsx

+type Alert = {
+  key: AlertKey;
+  label: string;
+  icon: string;
+};
+
+type AlertType = {
+  key: AlertTypeKey;
+  label: string;
+  icon: string;
+  subalerts: Alert[];
+};


isn't one type enough with the subalerts property being optional? (or just an empty array)

marinovl7

Sidebar Implementation is generally good, I really like it and overall awesome job. Just some refactoring would be needed to make it perfect, but no coding mistake here, just few code style guidelines which if you follow will make the component cleaner and easily maintanable.

marinovl7 · 2024-11-04T18:23:14Z

src/app/providers.tsx

+    <SidebarProvider>
+      <NextUIProvider navigate={router.push}>


SidebarProvider after the NextUIProvider

marinovl7 · 2024-11-04T18:24:07Z

src/components/AlertsMenu/AlertsMenu.tsx

+type Alert = {
+  key: AlertKey;
+  label: string;
+  icon: string;
+};
+
+type AlertType = {
+  key: AlertTypeKey;
+  label: string;
+  icon: string;
+  subalerts: Alert[];
+};


Types to be moved in domain/entities/sidebar

marinovl7 · 2024-11-04T18:25:11Z

src/components/AlertsMenu/AlertsMenu.tsx

+const alertTypes: (Alert | AlertType)[] = [
+  {
+    key: 'hunger',
+    label: 'Hunger Alert',
+    icon: '/menu_fcs.png',
+  },
+  {
+    key: 'conflicts',
+    label: 'Conflicts',
+    icon: '/menu_conflicts.png',
+    subalerts: [
+      {
+        key: 'conflicts1',
+        label: 'Conflicts 1',
+        icon: '/menu_conflicts.png',
+      },
+      {
+        key: 'conflicts2',
+        label: 'Conflicts 2',
+        icon: '/menu_conflicts.png',
+      },
+    ],
+  },
+  {
+    key: 'hazards',
+    label: 'Hazards',
+    icon: '/menu_hazards.png',
+  },
+];


This could be moved to operations/sidebar/SidebarOperations.ts

marinovl7 · 2024-11-04T18:25:21Z

src/components/AlertsMenu/AlertsMenu.tsx

+type AlertsMenuProps = {
+  variant: 'outside' | 'inside';
+};


Type same as above

marinovl7 · 2024-11-04T18:26:34Z

src/components/AlertsMenu/AlertsMenu.tsx

+              <Button isIconOnly radius="full" className={isSubAlertClicked(item.key) ? 'bg-primary' : 'bg-background'}>
+                <div className="w-6 h-6 flex items-center justify-center">
+                  <Image
+                    src={item.icon}
+                    alt={item.label}
+                    className="object-contain w-auto h-auto max-w-full max-h-full"
+                  />
+                </div>
+              </Button>


You should provide a function in the custom component props

marinovl7 · 2024-11-04T18:32:28Z

src/components/Sidebar/Sidebar.tsx

+              <p className="text-lg font-medium">HungerMap LIVE</p>
+            </div>
+            <Button isIconOnly variant="light" onClick={toggleSidebar} aria-label="Close sidebar">
+              <SquareChevronLeft size={18} />


size 18 in expanded vs size 24 for expanded is inconsistent

marinovl7 · 2024-11-04T18:34:04Z

src/components/Sidebar/Sidebar.tsx

+              <ListboxItem key="gloss">Glossary</ListboxItem>
+              <ListboxItem key="methd">Methodology</ListboxItem>
+              <ListboxItem key="disc">Disclaimer</ListboxItem>
+            </Listbox>


use entire words for the keys as well

marinovl7 · 2024-11-04T18:34:27Z

src/components/Sidebar/SidebarContext.tsx

+interface SelectedMapTypeState {
+  selectedMapType: MapKey;
+  setSelectedMapType: (value: MapKey) => void;
+}
+
+interface SelectedAlertsState {
+  selectedAlert: string;
+  setSelectedAlert: (value: string) => void;
+  isAlertSelected: (alertType: string) => boolean;
+  toggleAlert: (alertType: string) => void;
+}
+
+interface SidebarState {
+  isSidebarOpen: boolean;
+  toggleSidebar: () => void;
+  openSidebar: () => void;
+  closeSidebar: () => void;
+}


interfaces go to domain/entries/Sidebar

I moved SidebarContext.tsx to domain/contexts/SidebarContext.tsx. I left the intefaces in the file like it is in CountryContext.tsx

marinovl7 · 2024-11-04T18:34:57Z

src/components/Sidebar/Sidebar.tsx

+
+const mapTypes: MapType[] = [
+  {
+    key: 'food',


keys here generally should be an Enum

marinovl7 · 2024-11-04T18:36:21Z

src/components/AlertsMenu/AlertsMenu.tsx

+
+const alertTypes: (Alert | AlertType)[] = [
+  {
+    key: 'hunger',


keys should generally be Enums

export enum AlertTypes { HUNGER = 'hunger' ... ]

And use it like this

key: AlertType.HUNGER

…alert management

georgi4444 · 2024-11-07T20:07:24Z

All requested changes are done in a11194c

netlify · 2024-11-08T17:40:45Z

✅ Deploy Preview for wfp-hungermap ready!

Name	Link
🔨 Latest commit	`f3a026b`
🔍 Latest deploy log	https://app.netlify.com/sites/wfp-hungermap/deploys/672e4ee2e2e5c500084d771c
😎 Deploy Preview	https://deploy-preview-11--wfp-hungermap.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

marinovl7

Great Job!

georgi4444 changed the title ~~feat: implement sidebar~~ feat: initial sidebar implementation Nov 4, 2024

georgi4444 marked this pull request as ready for review November 4, 2024 14:14

georgi4444 requested review from Tschonti and marinovl7 November 4, 2024 14:14

Tschonti reviewed Nov 4, 2024

View reviewed changes

marinovl7 requested changes Nov 4, 2024

View reviewed changes

georgi4444 added 5 commits November 6, 2024 12:08

feat: inital sidebar implementation

6a2ab08

feat: update sidebar implementation and add alerts menu

2362dc8

feat: add AlertsMenuWrapper component and update sidebar context for …

9556032

…alert management

feat: improve layout and spacing for Alerts section

13dfdcf

feat: requested changes

a11194c

georgi4444 force-pushed the feature/f-29-implement-sidebar branch from 07e85c8 to a11194c Compare November 7, 2024 19:59

georgi4444 requested review from Tschonti and marinovl7 November 7, 2024 20:08

fix: theme switcher alignment

00d49d1

fix: solve merge conflicts

f3a026b

marinovl7 approved these changes Nov 8, 2024

View reviewed changes

marinovl7 merged commit 3386164 into main Nov 8, 2024
5 checks passed

marinovl7 deleted the feature/f-29-implement-sidebar branch November 8, 2024 17:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: initial sidebar implementation #11

feat: initial sidebar implementation #11

georgi4444 commented Nov 4, 2024 •

edited

Loading

Tschonti left a comment

Tschonti Nov 4, 2024

georgi4444 Nov 4, 2024

marinovl7 Nov 4, 2024

georgi4444 Nov 7, 2024

Tschonti Nov 4, 2024

marinovl7 left a comment

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

georgi4444 Nov 7, 2024

marinovl7 Nov 4, 2024

marinovl7 Nov 4, 2024

georgi4444 commented Nov 7, 2024

netlify bot commented Nov 8, 2024 •

edited

Loading

marinovl7 left a comment

feat: initial sidebar implementation #11

feat: initial sidebar implementation #11

Conversation

georgi4444 commented Nov 4, 2024 • edited Loading

Tschonti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marinovl7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgi4444 commented Nov 7, 2024

netlify bot commented Nov 8, 2024 • edited Loading

✅ Deploy Preview for wfp-hungermap ready!

marinovl7 left a comment

Choose a reason for hiding this comment

georgi4444 commented Nov 4, 2024 •

edited

Loading

netlify bot commented Nov 8, 2024 •

edited

Loading