From b318daf34f1e5ec0cbc9e51062d113eafc6ac249 Mon Sep 17 00:00:00 2001
From: Fikri Karim <fkfikrikarim@gmail.com>
Date: Mon, 30 Dec 2024 21:14:29 +0700
Subject: [PATCH] feat: add height prop to header and footer

This commits also updates:
- Make the Shape prop required. Makeswift supports this by default.
- Add default values to height and weight that matches the Soul default values.
- Move the banner id prop to be after allowClose.
- Rename and move some variables.
---
 .../site-footer/site-footer.client.tsx        | 15 ++++++-----
 .../site-footer/site-footer.makeswift.tsx     |  3 ++-
 .../components/site-footer/site-footer.tsx    |  4 +--
 .../site-header/site-header.client.tsx        | 26 ++++++++++---------
 .../site-header/site-header.makeswift.tsx     |  8 +++---
 5 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/core/lib/makeswift/components/site-footer/site-footer.client.tsx b/core/lib/makeswift/components/site-footer/site-footer.client.tsx
index d9e43d9736..60a935f799 100644
--- a/core/lib/makeswift/components/site-footer/site-footer.client.tsx
+++ b/core/lib/makeswift/components/site-footer/site-footer.client.tsx
@@ -32,10 +32,11 @@ export const PropsContextProvider = ({
 );
 
 interface Props {
-  logo?: {
+  logo: {
     show: boolean;
     src?: string;
-    width?: number;
+    width: number;
+    height: number;
     alt: string;
   };
   sections: Array<{
@@ -63,17 +64,17 @@ function combineSections(
 }
 
 export const MakeswiftFooter = forwardRef(
-  ({ logo: _logo, sections, copyright }: Props, ref: Ref<HTMLDivElement>) => {
+  ({ logo, sections, copyright }: Props, ref: Ref<HTMLDivElement>) => {
     const passedProps = useContext(PropsContext);
-    const logoObject = _logo?.src ? { src: _logo.src, alt: _logo.alt } : passedProps.logo;
-    const logo = _logo?.show ? logoObject : undefined;
+    const logoObject = logo.src ? { src: logo.src, alt: logo.alt } : passedProps.logo;
 
     return (
       <Footer
         {...passedProps}
         copyright={copyright ?? passedProps.copyright}
-        logo={logo}
-        logoWidth={_logo?.width ?? passedProps.logoWidth}
+        logo={logo.show ? logoObject : undefined}
+        logoHeight={logo.show ? logo.height : 0}
+        logoWidth={logo.show ? logo.width : 0}
         ref={ref}
         sections={combineSections(passedProps.sections, sections)}
       />
diff --git a/core/lib/makeswift/components/site-footer/site-footer.makeswift.tsx b/core/lib/makeswift/components/site-footer/site-footer.makeswift.tsx
index e298aa908c..70af3a86c3 100644
--- a/core/lib/makeswift/components/site-footer/site-footer.makeswift.tsx
+++ b/core/lib/makeswift/components/site-footer/site-footer.makeswift.tsx
@@ -15,7 +15,8 @@ runtime.registerComponent(MakeswiftFooter, {
       type: {
         show: Checkbox({ label: 'Show logo', defaultValue: true }),
         src: Image({ label: 'Logo' }),
-        width: Number({ label: 'Logo width', suffix: 'px' }),
+        width: Number({ label: 'Logo width', suffix: 'px', defaultValue: 200 }),
+        height: Number({ label: 'Logo height', suffix: 'px', defaultValue: 40 }),
         alt: TextInput({ label: 'Alt text', defaultValue: 'Logo alt' }),
       },
     }),
diff --git a/core/lib/makeswift/components/site-footer/site-footer.tsx b/core/lib/makeswift/components/site-footer/site-footer.tsx
index 3aec64a180..c9e468bd40 100644
--- a/core/lib/makeswift/components/site-footer/site-footer.tsx
+++ b/core/lib/makeswift/components/site-footer/site-footer.tsx
@@ -15,11 +15,11 @@ type Props = ComponentPropsWithoutRef<typeof VibesFooter> & {
 export const SiteFooter = async ({
   snapshotId = 'site-footer',
   label = 'Site Footer',
-  sections: _sections,
+  sections: streamableSections,
   ...props
 }: Props) => {
   const snapshot = await getComponentSnapshot(snapshotId);
-  const sections = await _sections;
+  const sections = await streamableSections;
 
   return (
     <PropsContextProvider value={{ ...props, sections }}>
diff --git a/core/lib/makeswift/components/site-header/site-header.client.tsx b/core/lib/makeswift/components/site-header/site-header.client.tsx
index 0182d7d5c5..f773ef6158 100644
--- a/core/lib/makeswift/components/site-header/site-header.client.tsx
+++ b/core/lib/makeswift/components/site-header/site-header.client.tsx
@@ -40,7 +40,7 @@ export const PropsContextProvider = ({
 );
 
 interface Props {
-  banner?: {
+  banner: {
     id: string;
     show: boolean;
     allowClose: boolean;
@@ -58,13 +58,15 @@ interface Props {
       }>;
     }>;
   }>;
-  logo?: {
+  logo: {
     desktopSrc?: string;
     desktopAlt: string;
-    desktopWidth?: number;
+    desktopWidth: number;
+    desktopHeight: number;
     mobileSrc?: string;
     mobileAlt: string;
-    mobileWidth?: number;
+    mobileWidth: number;
+    mobileHeight: number;
     link?: { href: string };
   };
   linksPosition: 'center' | 'left' | 'right';
@@ -91,7 +93,7 @@ function combineLinks(
 export const MakeswiftHeader = forwardRef(
   ({ banner, links, logo, linksPosition }: Props, ref: Ref<HTMLDivElement>) => {
     const { navigation: passedProps, banner: passedBanner } = useContext(PropsContext);
-    const combinedBanner = banner?.show
+    const combinedBanner = banner.show
       ? {
           ...passedBanner,
           id: banner.id,
@@ -106,16 +108,16 @@ export const MakeswiftHeader = forwardRef(
         navigation={{
           ...passedProps,
           links: combineLinks(passedProps.links, links),
-          logo: logo?.desktopSrc
-            ? { src: logo.desktopSrc, alt: logo.desktopAlt }
-            : passedProps.logo,
-          logoWidth: logo?.desktopWidth,
-          mobileLogo: logo?.mobileSrc
+          logo: logo.desktopSrc ? { src: logo.desktopSrc, alt: logo.desktopAlt } : passedProps.logo,
+          logoWidth: logo.desktopWidth,
+          logoHeight: logo.desktopHeight,
+          mobileLogo: logo.mobileSrc
             ? { src: logo.mobileSrc, alt: logo.mobileAlt }
             : passedProps.mobileLogo,
-          mobileLogoWidth: logo?.mobileWidth,
+          mobileLogoWidth: logo.mobileWidth,
+          mobileLogoHeight: logo.mobileHeight,
           linksPosition,
-          logoHref: logo?.link?.href ?? passedProps.logoHref,
+          logoHref: logo.link?.href ?? passedProps.logoHref,
         }}
         ref={ref}
       />
diff --git a/core/lib/makeswift/components/site-header/site-header.makeswift.tsx b/core/lib/makeswift/components/site-header/site-header.makeswift.tsx
index c0070b0175..399b1ca5c0 100644
--- a/core/lib/makeswift/components/site-header/site-header.makeswift.tsx
+++ b/core/lib/makeswift/components/site-header/site-header.makeswift.tsx
@@ -23,9 +23,9 @@ runtime.registerComponent(MakeswiftHeader, {
   props: {
     banner: Shape({
       type: {
-        id: TextInput({ label: 'Banner ID', defaultValue: 'black_friday_2025' }),
         show: Checkbox({ label: 'Show banner', defaultValue: false }),
         allowClose: Checkbox({ label: 'Allow banner to close', defaultValue: true }),
+        id: TextInput({ label: 'Banner ID', defaultValue: 'black_friday_2025' }),
         children: Slot(),
       },
     }),
@@ -33,10 +33,12 @@ runtime.registerComponent(MakeswiftHeader, {
       type: {
         desktopSrc: Image({ label: 'Desktop logo' }),
         desktopAlt: TextInput({ label: 'Desktop logo alt', defaultValue: 'Desktop logo alt' }),
-        desktopWidth: Number({ label: 'Desktop logo width', suffix: 'px' }),
+        desktopWidth: Number({ label: 'Desktop logo width', suffix: 'px', defaultValue: 200 }),
+        desktopHeight: Number({ label: 'Desktop logo height', suffix: 'px', defaultValue: 40 }),
         mobileSrc: Image({ label: 'Mobile logo' }),
         mobileAlt: TextInput({ label: 'Mobile logo alt', defaultValue: 'Mobile logo alt' }),
-        mobileWidth: Number({ label: 'Mobile logo width', suffix: 'px' }),
+        mobileWidth: Number({ label: 'Mobile logo width', suffix: 'px', defaultValue: 100 }),
+        mobileHeight: Number({ label: 'Mobile logo height', suffix: 'px', defaultValue: 40 }),
         link: Link({ label: 'Logo link' }),
       },
     }),