Skip to content

Tshepo/f/drawer component #3197

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.

Sign up for GitHub

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

Merged
James-Baloyi merged 3 commits into from
Apr 29, 2025
Merged

Tshepo/f/drawer component #3197

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df18262
chore: supporting the height and width on all slide directions
Tshepo1103-lab Apr 25, 2025
9c486d4
chore: changing to use the refactored styles
Tshepo1103-lab Apr 25, 2025
c5dd38d
Merge branch 'main' of https://github.com/Tshepo1103-lab/shesha-frame…
Tshepo1103-lab Apr 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions shesha-reactjs/src/designer-components/drawer/drawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { executeScriptSync, useAvailableConstantsData } from '@/providers/form/u
import { IConfigurableActionConfiguration } from '@/interfaces/configurableAction';
import { useConfigurableAction, useConfigurableActionDispatcher } from '@/providers/configurableActionsDispatcher';
import { IConfigurableFormComponent } from '@/providers';

interface IShaDrawer {
id?: string;
componentName?: string;
Expand All @@ -27,7 +26,12 @@ interface IShaDrawer {
placement?: 'top' | 'right' | 'bottom' | 'left';
width?: string | number;
readOnly?: boolean;
background?: CSSProperties;
backgroundStyles?: CSSProperties;
dimensions?: {
width?: string | number;
height?: string | number;
};
stylingBoxAsCSS?: CSSProperties;
}
Comment on lines +29 to 35
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Old & new props co-exist ⇒ API ambiguity

width (legacy) and dimensions.width (new) now sit side-by-side.
Keeping both will confuse users and complicate maintenance. Deprecate one path explicitly (JSDoc or runtime warning) and remove it in the next major.


interface IShaDrawerState {
Expand All @@ -38,7 +42,7 @@ const ShaDrawer: FC<IShaDrawer> = (props) => {
const {
id,
placement,
width,
dimensions,
componentName: name,
readOnly,
label,
Expand All @@ -55,11 +59,13 @@ const ShaDrawer: FC<IShaDrawer> = (props) => {
headerStyle,
footerStyle,
showFooter,
background,
backgroundStyles,
stylingBoxAsCSS,
} = props;
const allData = useAvailableConstantsData();
const [state, setState] = useState<IShaDrawerState>();
const { executeAction } = useConfigurableActionDispatcher();
const { paddingTop, paddingRight, paddingBottom, paddingLeft, ...rest } = stylingBoxAsCSS;

Comment on lines +68 to 69
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Possible destructuring crash when stylingBoxAsCSS is undefined

If the prop is omitted, destructuring throws. Guard with a default:

-  const { paddingTop, paddingRight, paddingBottom, paddingLeft, ...rest } = stylingBoxAsCSS;
+  const {
+    paddingTop,
+    paddingRight,
+    paddingBottom,
+    paddingLeft,
+    ...rest
+  } = stylingBoxAsCSS ?? {};
📝 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.

Suggested change
const { paddingTop, paddingRight, paddingBottom, paddingLeft, ...rest } = stylingBoxAsCSS;
const {
paddingTop,
paddingRight,
paddingBottom,
paddingLeft,
...rest
} = stylingBoxAsCSS ?? {};

const openDrawer = () => setState((prev) => ({ ...prev, open: true }));

Expand Down Expand Up @@ -145,13 +151,27 @@ const ShaDrawer: FC<IShaDrawer> = (props) => {
<Drawer
open={state?.open}
placement={placement}
width={width}
width={dimensions?.width}
height={dimensions?.height}
onClose={closeDrawer}
styles={{
header: { display: showHeader ? 'block' : 'none', ...headerStyle },
footer: { display: showFooter ? 'block' : 'none', ...footerStyle },
body: background,
content: style,
body: backgroundStyles as CSSProperties,
content: {
...style,
height: undefined,
width: undefined,
paddingTop,
paddingRight,
paddingBottom,
paddingLeft,
},
wrapper: {
width: style?.width || props.dimensions?.width,
height: style?.height || props.dimensions?.height,
...rest,
},
}}
title={label}
size="large"
Expand Down
145 changes: 43 additions & 102 deletions shesha-reactjs/src/designer-components/drawer/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,119 +1,47 @@
import React, { CSSProperties, useEffect, useMemo, useState } from 'react';
import { migrateNavigateAction } from '@/designer-components/_common-migrations/migrate-navigate-action';
import { migrateCustomFunctions, migratePropertyName } from '@/designer-components/_common-migrations/migrateSettings';
import { useFormComponentStyles } from '@/hooks/formComponentHooks';
import { IToolboxComponent } from '@/interfaces';
import { IInputStyles } from '@/providers';
import { validateConfigurableComponentSettings } from '@/providers/form/utils';
import { removeUndefinedProps } from '@/utils/object';
import { SwapOutlined } from '@ant-design/icons';
import React, { CSSProperties } from 'react';
import { migrateFormApi } from '../_common-migrations/migrateFormApi1';
import { migratePrevStyles } from '../_common-migrations/migrateStyles';
import ShaDrawer from './drawer';
import { IDrawerProps } from './models';
import { getStyle, pickStyleFromModel, validateConfigurableComponentSettings } from '@/providers/form/utils';
import { migrateCustomFunctions, migratePropertyName } from '@/designer-components/_common-migrations/migrateSettings';
import { IInputStyles, useFormData, useSheshaApplication } from '@/providers';
import { migrateNavigateAction } from '@/designer-components/_common-migrations/migrate-navigate-action';
import { migrateFormApi } from '../_common-migrations/migrateFormApi1';
import { getSettings } from './settingsForm';
import { removeUndefinedProps } from '@/utils/object';
import { getBackgroundImageUrl, getBackgroundStyle } from '../_settings/utils/background/utils';
import { ValidationErrors } from '@/components';
import { isValidGuid } from '@/components/formDesigner/components/utils';
import { getBorderStyle } from '../_settings/utils/border/utils';
import { migratePrevStyles } from '../_common-migrations/migrateStyles';
import { getShadowStyle } from '../_settings/utils/shadow/utils';
import { defaultStyles, initialStyle } from './utils';
import { defaultStyles } from './utils';

const DrawerComponent: IToolboxComponent<IDrawerProps> = {
type: 'drawer',
isInput: false,
name: 'Drawer',
icon: <SwapOutlined />,
Factory: ({ model }) => {
const { data } = useFormData();
const { backendUrl, httpHeaders } = useSheshaApplication();
const {
size,
border,
background,
headerBackground,
style,
shadow,
headerShadow,
headerStyle,
footerStyle,
footerBackground,
footerShadow,
...props
} = model;

const jsStyle = getStyle(style, data);
const [backgroundStyles, setBackgroundStyles] = useState({});
const borderStyles = useMemo(() => getBorderStyle(border, jsStyle), [border]);
const shadowStyles = useMemo(() => getShadowStyle(shadow), [shadow]);

const headerJsStyle = getStyle(headerStyle, data);
const [headerBackgroundStyles, setHeaderBackgroundStyles] = useState({});
const headerShadowStyles = useMemo(() => getShadowStyle(headerShadow), [headerShadow]);

const footerJsStyle = getStyle(footerStyle, data);
const footerShadowStyles = useMemo(() => getShadowStyle(footerShadow), [footerShadow]);
const [footerBackgroundStyles, setFooterBackgroundStyles] = useState({});

useEffect(() => {
const fetchStyles = async () => {
getBackgroundImageUrl(background, backendUrl, httpHeaders)
.then(async (url) => {
return await getBackgroundStyle(background, jsStyle, url);
})
.then((style) => {
setBackgroundStyles(style);
});
};
fetchStyles();
}, [background, backendUrl, httpHeaders]);
const { allStyles, style, headerStyles, footerStyles, ...props } = model;

useEffect(() => {
const fetchStyles = async () => {
getBackgroundImageUrl(headerBackground, backendUrl, httpHeaders)
.then(async (url) => {
return await getBackgroundStyle(headerBackground, headerJsStyle, url);
})
.then((style) => {
setHeaderBackgroundStyles(style);
});
};
fetchStyles();
}, [headerBackground, backendUrl, httpHeaders]);

useEffect(() => {
const fetchStyles = async () => {
getBackgroundImageUrl(footerBackground, backendUrl, httpHeaders)
.then(async (url) => {
return await getBackgroundStyle(footerBackground, footerJsStyle, url);
})
.then((style) => {
setFooterBackgroundStyles(style);
});
};
fetchStyles();
}, [footerBackground, backendUrl, httpHeaders]);

if (
(model?.background?.type === 'storedFile' &&
model?.background.storedFile?.id &&
!isValidGuid(model?.background.storedFile.id)) ||
(model?.headerBackground?.type === 'storedFile' &&
model?.headerBackground.storedFile?.id &&
!isValidGuid(model?.headerBackground.storedFile.id)) ||
(model?.footerBackground?.type === 'storedFile' &&
model?.footerBackground.storedFile?.id &&
!isValidGuid(model?.footerBackground.storedFile.id))
) {
return <ValidationErrors error="The provided StoredFileId is invalid" />;
}
const {
backgroundStyles: headerBackgroundStyles,
shadowStyles: headerShadowStyles,
jsStyle: headerJsStyle,
} = useFormComponentStyles(headerStyles);
const {
backgroundStyles: footerBackgroundStyles,
shadowStyles: footerShadowStyles,
jsStyle: footerJsStyle,
} = useFormComponentStyles(footerStyles);
const jsStyle = allStyles?.jsStyle;
const stylingBoxAsCSS = allStyles?.stylingBoxAsCSS;

const styling = JSON.parse(model.stylingBox || '{}');
const stylingBoxAsCSS = pickStyleFromModel(styling);
const borderStyles = allStyles?.borderStyles;
const shadowStyles = allStyles?.shadowStyles;

const additionalStyles: CSSProperties = removeUndefinedProps({
...shadowStyles,
...stylingBoxAsCSS,
...borderStyles,
stylingBoxAsCSS,
...jsStyle,
});
Comment on lines 41 to 46
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

stylingBoxAsCSS is passed twice and pollutes inline CSS

stylingBoxAsCSS is already forwarded as an explicit prop (line 62) yet it is also embedded inside the style object – thereby producing an invalid CSS property and risking React warnings.

-const additionalStyles: CSSProperties = removeUndefinedProps({
-  ...shadowStyles,
-  ...borderStyles,
-  stylingBoxAsCSS,
-  ...jsStyle,
-});
+const additionalStyles: CSSProperties = removeUndefinedProps({
+  ...shadowStyles,
+  ...borderStyles,
+  ...jsStyle,
+});

A single source of truth avoids duplication and keeps the generated style object valid.

📝 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.

Suggested change
const additionalStyles: CSSProperties = removeUndefinedProps({
...shadowStyles,
...stylingBoxAsCSS,
...borderStyles,
stylingBoxAsCSS,
...jsStyle,
});
const additionalStyles: CSSProperties = removeUndefinedProps({
...shadowStyles,
...borderStyles,
...jsStyle,
});


Expand All @@ -131,7 +59,8 @@ const DrawerComponent: IToolboxComponent<IDrawerProps> = {

return (
<ShaDrawer
background={backgroundStyles}
stylingBoxAsCSS={stylingBoxAsCSS}
backgroundStyles={allStyles?.backgroundStyles}
style={additionalStyles}
headerStyle={additionalHeaderStyles}
footerStyle={additionalFooterStyles}
Expand Down Expand Up @@ -167,9 +96,21 @@ const DrawerComponent: IToolboxComponent<IDrawerProps> = {
})
.add<IDrawerProps>(4, (prev) => ({
...migratePrevStyles(prev, defaultStyles()),
desktop: { ...migratePrevStyles(prev, defaultStyles()).desktop, ...initialStyle },
tablet: { ...migratePrevStyles(prev, defaultStyles()).tablet, ...initialStyle },
mobile: { ...migratePrevStyles(prev, defaultStyles()).mobile, ...initialStyle },
desktop: {
...migratePrevStyles(prev, defaultStyles()).desktop,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
tablet: {
...migratePrevStyles(prev, defaultStyles()).tablet,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
mobile: {
...migratePrevStyles(prev, defaultStyles()).mobile,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
})),
Comment on lines 97 to 114
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Migrate only once to reduce computation & dodge subtle divergence

migratePrevStyles(prev, defaultStyles()) is executed four times, producing four independent objects that can diverge if defaultStyles() ever becomes non-deterministic (timestamp, random IDs, etc.). It is also wasteful.

-      .add<IDrawerProps>(4, (prev) => ({
-        ...migratePrevStyles(prev, defaultStyles()),
-        desktop: {
-          ...migratePrevStyles(prev, defaultStyles()).desktop,
+      .add<IDrawerProps>(4, (prev) => {
+        const migrated = migratePrevStyles(prev, defaultStyles());
+        return {
+          ...migrated,
+          desktop: {
+            ...migrated.desktop,
             headerStyles: defaultStyles(),
             footerStyles: defaultStyles(),
           },
           tablet: {
-          ...migratePrevStyles(prev, defaultStyles()).tablet,
+            ...migrated.tablet,
             headerStyles: defaultStyles(),
             footerStyles: defaultStyles(),
           },
           mobile: {
-          ...migratePrevStyles(prev, defaultStyles()).mobile,
+            ...migrated.mobile,
             headerStyles: defaultStyles(),
             footerStyles: defaultStyles(),
           },
-        },
-      })),
+        };
+      }))

This single evaluation improves performance and guarantees consistency across breakpoints.

📝 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.

Suggested change
.add<IDrawerProps>(4, (prev) => ({
...migratePrevStyles(prev, defaultStyles()),
desktop: { ...migratePrevStyles(prev, defaultStyles()).desktop, ...initialStyle },
tablet: { ...migratePrevStyles(prev, defaultStyles()).tablet, ...initialStyle },
mobile: { ...migratePrevStyles(prev, defaultStyles()).mobile, ...initialStyle },
desktop: {
...migratePrevStyles(prev, defaultStyles()).desktop,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
tablet: {
...migratePrevStyles(prev, defaultStyles()).tablet,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
mobile: {
...migratePrevStyles(prev, defaultStyles()).mobile,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
})),
.add<IDrawerProps>(4, (prev) => {
const migrated = migratePrevStyles(prev, defaultStyles());
return {
...migrated,
desktop: {
...migrated.desktop,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
tablet: {
...migrated.tablet,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
mobile: {
...migrated.mobile,
headerStyles: defaultStyles(),
footerStyles: defaultStyles(),
},
};
}))

initModel: (model) => {
const customProps: IDrawerProps = {
Expand Down
22 changes: 5 additions & 17 deletions shesha-reactjs/src/designer-components/drawer/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { IConfigurableActionConfiguration } from '@/interfaces/configurableAction';
import { IConfigurableFormComponent } from '@/interfaces/formDesigner';
import { IBackgroundValue } from '../_settings/utils/background/interfaces';
import { IBorderValue } from '../_settings/utils/border/interfaces';
import { IShadowValue } from '../_settings/utils/shadow/interfaces';
import { IFontValue } from '../_settings/utils/font/interfaces';
import { IInputStyles } from '@/index';
import { IBackgroundValue } from '../_settings/utils/background/interfaces';

export interface IDrawerProps extends IConfigurableFormComponent {
showFooter?: boolean;
Expand All @@ -23,8 +20,6 @@ export interface IDrawerProps extends IConfigurableFormComponent {
placement?: 'top' | 'right' | 'bottom' | 'left';
height?: string | number;
width?: string | number;
background?: IBackgroundValue;
border?: IBorderValue;
hideBorder?: boolean;
stylingBox?: string;
borderSize?: number;
Expand All @@ -33,19 +28,12 @@ export interface IDrawerProps extends IConfigurableFormComponent {
fontColor?: string;
backgroundColor?: string;
fontSize?: number;
headerStyle?: string;
footerStyle?: string;
shadow?: IShadowValue;
headerShadow?: IShadowValue;
headerBackground?: IBackgroundValue;
font?: IFontValue;
footerShadow?: IShadowValue;
footerBackground?: IBackgroundValue;
headerStyles?: IDrawerProps;
footerStyles?: any;
background?: IBackgroundValue;

Comment on lines +31 to +34
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid self-referential type explosion & restore type-safety

headerStyles recursively points to the entire IDrawerProps interface and footerStyles is typed as any.
This opens two problems:

  1. Infinite depth – any JSON–serialisation / deep-clone util walking the object tree could hit an endless loop or stack overflow because every headerStyles again contains its own headerStyles, and so on.
  2. Loss of compile-time guarantees for footerStyles.

Introduce a dedicated, flat style interface (e.g. IDrawerSectionStyles) that only exposes the properties really needed by the header/footer. Then reference it in both places.

-  headerStyles?: IDrawerProps;
-  footerStyles?: any;
+  headerStyles?: IDrawerSectionStyles;
+  footerStyles?: IDrawerSectionStyles;

Don’t forget to export the new interface and update usages in index.tsx, the migrator and the settings form.

📝 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.

Suggested change
headerStyles?: IDrawerProps;
footerStyles?: any;
background?: IBackgroundValue;
headerStyles?: IDrawerSectionStyles;
footerStyles?: IDrawerSectionStyles;
background?: IBackgroundValue;

desktop?: IInputStyles;
tablet?: IInputStyles;
mobile?: IInputStyles;



components?: IConfigurableFormComponent[];
}
Loading