Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
31 changes: 31 additions & 0 deletions shesha-reactjs/src/designer-components/dataTable/table/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,39 @@ export interface ITableComponentBaseProps extends IShaDataTableInlineEditablePro
rowAlternateBackgroundColor?: string;
rowHoverBackgroundColor?: string;
rowSelectedBackgroundColor?: string;

// Row dimensions
rowDimensions?: {
height?: string;
minHeight?: string;
maxHeight?: string;
};

// Row padding using styling box
rowStylingBox?: {
margin?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
padding?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
};

// Row border using standard border controls
rowBorderStyle?: IBorderValue;

// Deprecated properties - kept for backward compatibility
/** @deprecated Use rowDimensions.height instead */
rowHeight?: string;
/** @deprecated Use rowStylingBox.padding instead */
rowPadding?: string;
/** @deprecated Use rowBorderStyle instead */
rowBorder?: string;

// Overall table styling
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ const TableComponent: TableComponentDefinition = {
actualModelPropertyFilter: (name, value) => {
// Allow all styling properties through to the settings form
const allowedStyleProperties = [
// Old properties (deprecated but kept for backward compatibility)
'rowHeight', 'rowPadding', 'rowBorder',
// New structured properties
'rowDimensions', 'rowStylingBox', 'rowBorderStyle',
'headerFontSize', 'headerFontWeight',
'tableSettings', // For nested structure
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,29 +555,94 @@ export const getSettings: SettingsFormMarkupFactory = ({ fbf }) => {
content: {
id: nanoid(),
components: [...fbf()
.addSettingsInput({
.addCollapsiblePanel({
id: nanoid(),
propertyName: 'rowHeight',
label: 'Row Height',
inputType: 'textField',
tooltip: 'Height for table rows (e.g., 40px, 3rem)',
jsSetting: false,
propertyName: 'pnlRowDimensions',
label: 'Row Dimensions',
labelAlign: 'right',
ghost: true,
collapsible: 'header',
content: {
id: nanoid(),
components: [...fbf()
.addSettingsInputRow({
id: nanoid(),
inline: true,
inputs: [
{
type: 'textField',
id: nanoid(),
label: "Height",
width: 85,
propertyName: "rowDimensions.height",
icon: "heightIcon",
tooltip: "Row height. You can use any unit (%, px, em, etc). px by default if without unit",
},
{
type: 'textField',
id: nanoid(),
label: "Min Height",
width: 85,
hideLabel: true,
propertyName: "rowDimensions.minHeight",
icon: "minHeightIcon",
},
{
type: 'textField',
id: nanoid(),
label: "Max Height",
width: 85,
hideLabel: true,
propertyName: "rowDimensions.maxHeight",
icon: "maxHeightIcon",
},
],
})
.toJson(),
],
},
})
.addSettingsInput({
.addCollapsiblePanel({
id: nanoid(),
propertyName: 'rowPadding',
label: 'Cell Padding',
inputType: 'textField',
tooltip: 'Padding for table cells (e.g., 8px, 0.5rem 1rem)',
jsSetting: false,
propertyName: 'rowStylingBox',
label: 'Row Padding',
labelAlign: 'right',
ghost: true,
collapsible: 'header',
content: {
id: nanoid(),
components: [...fbf()
.addStyleBox({
id: nanoid(),
label: 'Cell Padding',
hideLabel: true,
propertyName: 'rowStylingBox',
})
.toJson(),
],
},
})
.addSettingsInput({
.addCollapsiblePanel({
id: nanoid(),
propertyName: 'rowBorder',
propertyName: 'pnlRowBorderStyle',
label: 'Row Border',
inputType: 'textField',
tooltip: 'Border style for table rows (e.g., 1px solid #ccc)',
jsSetting: false,
labelAlign: 'right',
ghost: true,
collapsible: 'header',
content: {
id: nanoid(),
components: [...fbf()
.addContainer({
id: nanoid(),
components: getBorderInputs(fbf, 'rowBorder'),
})
.addContainer({
id: nanoid(),
components: getCornerInputs(fbf, 'rowBorderCorner'),
})
.toJson(),
],
},
})
.toJson(),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
useRef,
useEffect,
} from 'react';
import { filterVisibility, calculateDefaultColumns } from './utils';
import { filterVisibility, calculateDefaultColumns, convertRowDimensionsToHeight, convertRowStylingBoxToPadding, convertRowBorderStyleToBorder } from './utils';
import { getStyle } from '@/providers/form/utils';
import { ITableComponentProps } from './models';
import { getShadowStyle } from '@/designer-components/_settings/utils/shadow/utils';
Expand Down Expand Up @@ -67,6 +67,28 @@
return props?.shadow ? shadowStyles?.boxShadow : props?.boxShadow;
}, [props?.shadow, shadowStyles?.boxShadow, props?.boxShadow]);

// Convert new property structures to old format for backward compatibility
const effectiveRowHeight = useMemo(() => {
// Prefer new rowDimensions over old rowHeight
const converted = convertRowDimensionsToHeight(props?.rowDimensions);
console.log('Row Height - rowDimensions:', props?.rowDimensions, 'converted:', converted, 'fallback:', props?.rowHeight);

Check failure on line 74 in shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx

View workflow job for this annotation

GitHub Actions / build-attempt

Unexpected console statement. Only these console methods are allowed: warn, dir, timeLog, assert, clear, count, countReset, group, groupEnd, table, dirxml, error, groupCollapsed, Console, profile, profileEnd, timeStamp, context
return converted || props?.rowHeight;
}, [props?.rowDimensions, props?.rowHeight]);

const effectiveRowPadding = useMemo(() => {
// Prefer new rowStylingBox over old rowPadding
const converted = convertRowStylingBoxToPadding(props?.rowStylingBox);
console.log('Row Padding - rowStylingBox:', props?.rowStylingBox, 'converted:', converted, 'fallback:', props?.rowPadding);

Check failure on line 81 in shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx

View workflow job for this annotation

GitHub Actions / build-attempt

Unexpected console statement. Only these console methods are allowed: warn, dir, timeLog, assert, clear, count, countReset, group, groupEnd, table, dirxml, error, groupCollapsed, Console, profile, profileEnd, timeStamp, context
return converted || props?.rowPadding;
}, [props?.rowStylingBox, props?.rowPadding]);

const effectiveRowBorder = useMemo(() => {
// Prefer new rowBorderStyle over old rowBorder
const converted = convertRowBorderStyleToBorder(props?.rowBorderStyle);
console.log('Row Border - rowBorderStyle:', props?.rowBorderStyle, 'converted:', converted, 'fallback:', props?.rowBorder);

Check failure on line 88 in shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx

View workflow job for this annotation

GitHub Actions / build-attempt

Unexpected console statement. Only these console methods are allowed: warn, dir, timeLog, assert, clear, count, countReset, group, groupEnd, table, dirxml, error, groupCollapsed, Console, profile, profileEnd, timeStamp, context
return converted || props?.rowBorder;
}, [props?.rowBorderStyle, props?.rowBorder]);

const { styles } = useStyles({
fontFamily: props?.font?.type,
fontWeight: props?.font?.weight,
Expand All @@ -88,9 +110,9 @@
headerFontWeight: props?.headerFontWeight,
headerBackgroundColor: props?.headerBackgroundColor,
headerTextColor: props?.headerTextColor,
rowHeight: props?.rowHeight,
rowPadding: props?.rowPadding,
rowBorder: props?.rowBorder,
rowHeight: effectiveRowHeight,
rowPadding: effectiveRowPadding,
rowBorder: effectiveRowBorder,
boxShadow: finalBoxShadow,
sortableIndicatorColor: props?.sortableIndicatorColor,
});
Expand Down Expand Up @@ -359,9 +381,9 @@
headerFontWeight={props.headerFontWeight}
headerBackgroundColor={props.headerBackgroundColor}
headerTextColor={props.headerTextColor}
rowHeight={props.rowHeight}
rowPadding={props.rowPadding}
rowBorder={props.rowBorder}
rowHeight={effectiveRowHeight}
rowPadding={effectiveRowPadding}
rowBorder={effectiveRowBorder}
boxShadow={finalBoxShadow}
sortableIndicatorColor={props.sortableIndicatorColor}
/>
Expand Down
100 changes: 100 additions & 0 deletions shesha-reactjs/src/designer-components/dataTable/table/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,103 @@

return columnItems;
};

/**
* Adds 'px' unit to a value if it's a number without a unit
*/
const addPxUnit = (value?: string | number): string => {
if (!value && value !== 0) return '0px';
const strValue = String(value);
// If it's just a number (with optional decimal), add 'px'
if (/^-?\d+\.?\d*$/.test(strValue)) {
return `${strValue}px`;
}
return strValue;
};
Comment on lines +226 to +234
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider edge cases in numeric detection.

The regex pattern /^-?\d+\.?\d*$/ handles common numeric formats but would not match scientific notation (e.g., '1e10') or explicit positive signs (e.g., '+5'). While these are uncommon in CSS contexts, you may want to document this limitation or enhance the pattern if broader numeric support is needed.

If broader support is desired:

 const addPxUnit = (value?: string | number): string => {
   if (!value && value !== 0) return '0px';
   const strValue = String(value);
-  // If it's just a number (with optional decimal), add 'px'
-  if (/^-?\d+\.?\d*$/.test(strValue)) {
+  // If it's just a number (with optional decimal or scientific notation), add 'px'
+  if (/^[+-]?\d+\.?\d*([eE][+-]?\d+)?$/.test(strValue)) {
     return `${strValue}px`;
   }
   return strValue;
 };
📝 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 addPxUnit = (value?: string | number): string => {
if (!value && value !== 0) return '0px';
const strValue = String(value);
// If it's just a number (with optional decimal), add 'px'
if (/^-?\d+\.?\d*$/.test(strValue)) {
return `${strValue}px`;
}
return strValue;
};
const addPxUnit = (value?: string | number): string => {
if (!value && value !== 0) return '0px';
const strValue = String(value);
// If it's just a number (with optional decimal or scientific notation), add 'px'
if (/^[+-]?\d+\.?\d*([eE][+-]?\d+)?$/.test(strValue)) {
return `${strValue}px`;
}
return strValue;
};
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/dataTable/table/utils.ts around lines
226 to 234, the numeric-detection regex excludes values like "+5" and scientific
notation ("1e10"); either broaden the regex to accept an optional leading "+"
and exponent notation (e.g. allow /^\+?-?\d+(\.\d+)?([eE][+-]?\d+)?$/ or
equivalent) so those strings get "px" appended, or explicitly document the
current limitation in a brief comment above addPxUnit if you prefer to keep the
simpler pattern.


/**
* Converts row dimensions object to a single height string
* @param rowDimensions - Row dimensions configuration
* @returns Height string for backward compatibility
*/
export const convertRowDimensionsToHeight = (rowDimensions?: {
height?: string;
minHeight?: string;
maxHeight?: string;
}): string | undefined => {
if (!rowDimensions?.height) return undefined;
// Add px unit if it's just a number
return addPxUnit(rowDimensions.height);
};

/**
* Converts styling box padding to a single padding string
* @param rowStylingBox - Styling box configuration
* @returns Padding string for backward compatibility
*/
export const convertRowStylingBoxToPadding = (rowStylingBox?: {
margin?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
padding?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
}): string | undefined => {
if (!rowStylingBox?.padding) return undefined;

const { top, right, bottom, left } = rowStylingBox.padding;

// Add px units to values if needed
const topPx = addPxUnit(top);
const rightPx = addPxUnit(right);
const bottomPx = addPxUnit(bottom);
const leftPx = addPxUnit(left);

// If all values are the same, return single value
if (topPx === rightPx && rightPx === bottomPx && bottomPx === leftPx) {
return topPx;
}

// If top/bottom and left/right pairs are the same
if (topPx === bottomPx && leftPx === rightPx) {
return `${topPx} ${leftPx}`;
}

// Return all four values
return `${topPx} ${rightPx} ${bottomPx} ${leftPx}`;
};
Comment on lines +266 to +302
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Clarify or remove unused margin parameter.

The function signature includes a margin property in the rowStylingBox parameter, but this property is never referenced in the function body. Only the padding property is used for the conversion.

If margin is included for future extensibility or to match a shared type definition, consider adding a comment explaining this. Otherwise, remove it to improve clarity.

Apply this diff if margin is not needed:

 export const convertRowStylingBoxToPadding = (rowStylingBox?: {
-  margin?: {
-    top?: string;
-    right?: string;
-    bottom?: string;
-    left?: string;
-  };
   padding?: {
     top?: string;
     right?: string;
     bottom?: string;
     left?: string;
   };
 }): string | undefined => {
📝 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
export const convertRowStylingBoxToPadding = (rowStylingBox?: {
margin?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
padding?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
}): string | undefined => {
if (!rowStylingBox?.padding) return undefined;
const { top, right, bottom, left } = rowStylingBox.padding;
// Add px units to values if needed
const topPx = addPxUnit(top);
const rightPx = addPxUnit(right);
const bottomPx = addPxUnit(bottom);
const leftPx = addPxUnit(left);
// If all values are the same, return single value
if (topPx === rightPx && rightPx === bottomPx && bottomPx === leftPx) {
return topPx;
}
// If top/bottom and left/right pairs are the same
if (topPx === bottomPx && leftPx === rightPx) {
return `${topPx} ${leftPx}`;
}
// Return all four values
return `${topPx} ${rightPx} ${bottomPx} ${leftPx}`;
};
export const convertRowStylingBoxToPadding = (rowStylingBox?: {
padding?: {
top?: string;
right?: string;
bottom?: string;
left?: string;
};
}): string | undefined => {
if (!rowStylingBox?.padding) return undefined;
const { top, right, bottom, left } = rowStylingBox.padding;
// Add px units to values if needed
const topPx = addPxUnit(top);
const rightPx = addPxUnit(right);
const bottomPx = addPxUnit(bottom);
const leftPx = addPxUnit(left);
// If all values are the same, return single value
if (topPx === rightPx && rightPx === bottomPx && bottomPx === leftPx) {
return topPx;
}
// If top/bottom and left/right pairs are the same
if (topPx === bottomPx && leftPx === rightPx) {
return `${topPx} ${leftPx}`;
}
// Return all four values
return `${topPx} ${rightPx} ${bottomPx} ${leftPx}`;
};
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/dataTable/table/utils.ts around lines
266 to 302, the rowStylingBox parameter declares an unused margin property;
remove margin from the parameter type if it isn't required (change the type to
only include padding) and update any callers/types if necessary OR, if margin is
kept for type compatibility, add a one-line comment above the function
explaining margin is intentionally present for future/compatibility reasons and
leave the implementation as-is; ensure TypeScript types and imports remain
consistent after the change.


/**
* Converts border value object to a single border string
* @param rowBorderStyle - Border style configuration
* @returns Border string for backward compatibility
*/
export const convertRowBorderStyleToBorder = (rowBorderStyle?: any): string | undefined => {

Check failure on line 299 in shesha-reactjs/src/designer-components/dataTable/table/utils.ts

View workflow job for this annotation

GitHub Actions / build-attempt

Argument 'rowBorderStyle' should be typed with a non-any type
if (!rowBorderStyle?.border) return undefined;

const { borderType, border } = rowBorderStyle;

// If borderType is 'all', use the all configuration
if (borderType === 'all' && border.all) {
const { width, style, color } = border.all;
if (!width || !style || !color) return undefined;
return `${width} ${style} ${color}`;
}

// For custom borders, we can only return one border string
// Let's use the top border or the first available border
const firstBorder = border.top || border.right || border.bottom || border.left;
if (firstBorder) {
const { width, style, color } = firstBorder;
if (!width || !style || !color) return undefined;
return `${width} ${style} ${color}`;
}
Comment on lines +330 to +337
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Document the limitation of discarding custom border configurations.

When borderType is not 'all', the function only returns the first available border (trying top, right, bottom, left in order) and discards any other custom border configurations. This is a backward-compatibility limitation similar to the minHeight/maxHeight case in convertRowDimensionsToHeight.

Add JSDoc to document this limitation and recommend using rowBorderStyle directly when per-side borders are needed.

 /**
  * Converts border value object to a single border string
+ * 
+ * IMPORTANT LIMITATION: When borderType is not 'all', this function only returns
+ * the first available border (top, right, bottom, or left) and discards other
+ * custom border configurations. This is a backward-compatibility limitation for
+ * the legacy `rowBorder` property, which only supports a single border value.
+ * 
+ * If you need per-side border configurations, access the `rowBorderStyle` property
+ * directly instead of using this conversion helper.
+ * 
  * @param rowBorderStyle - Border style configuration
  * @returns Border string for backward compatibility
  */

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/dataTable/table/utils.ts around lines
330 to 337, the code picks only the first available side when constructing a
custom border and silently discards other side-specific configurations; add a
JSDoc comment above this block explaining this limitation (that when borderType
!== 'all' we return only one border string and ignore other side borders),
mention that this is a backward-compatibility constraint similar to
convertRowDimensionsToHeight, and recommend using rowBorderStyle directly when
per-side borders are required so callers know to avoid relying on per-side
border preservation.

Comment on lines +304 to +337
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Border conversion: allow width = 0 and document the “first side only” limitation.

Two small improvements will make this helper safer and clearer:

  1. width is treated as missing when it is 0 because of the falsy check, so a legitimate 0 width border will be discarded.
  2. The JSDoc doesn’t mention that for custom borders you only return the first available side and drop the rest, which is an intentional legacy limitation.

Recommend:

  • Switch the width check to explicit null/undefined.
  • Extend the JSDoc to document the first‑side‑only behavior.
-/**
- * Converts border value object to a single border string
- * @param rowBorderStyle - Border style configuration
- * @returns Border string for backward compatibility
- */
+/**
+ * Converts border value object to a single border string.
+ *
+ * IMPORTANT LIMITATION: When `borderType` is not `'all'`, this helper returns
+ * only the first available side (top, right, bottom, or left) and discards
+ * other side-specific border settings. This mirrors the legacy `rowBorder`
+ * prop, which only supports a single border value.
+ *
+ * Callers that need full per-side configuration should use `rowBorderStyle`
+ * directly instead of this legacy conversion helper.
+ *
+ * @param rowBorderStyle - Border style configuration
+ * @returns Border string for backward compatibility
+ */
 export const convertRowBorderStyleToBorder = (rowBorderStyle?: {
@@
   // If borderType is 'all', use the all configuration
   if (borderType === 'all' && border.all) {
     const { width, style, color } = border.all;
-    if (!width || !style || !color) return undefined;
+    if (width === undefined || width === null || !style || !color) return undefined;
     return `${addPxUnit(width)} ${style} ${color}`;
   }
@@
   const firstBorder = border.top || border.right || border.bottom || border.left;
   if (firstBorder) {
     const { width, style, color } = firstBorder;
-    if (!width || !style || !color) return undefined;
+    if (width === undefined || width === null || !style || !color) return undefined;
     return `${addPxUnit(width)} ${style} ${color}`;
   }
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/dataTable/table/utils.ts around lines
304 to 337, the helper treats width === 0 as missing due to falsy checks and the
JSDoc omits that for non-'all' borders we return only the first available side;
change the width existence checks from falsy checks to explicit null/undefined
checks (e.g. width === null || width === undefined) so a zero width is allowed,
and update the JSDoc comment to document that for custom borders the function
returns only the first available side (top → right → bottom → left) for backward
compatibility.


return undefined;
};
Comment on lines +251 to +340
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider extracting inline type definitions.

The three conversion functions (convertRowDimensionsToHeight, convertRowStylingBoxToPadding, convertRowBorderStyleToBorder) use inline type definitions for their parameters. Consider extracting these to named type definitions or interfaces for better reusability and maintainability, especially if these types are used elsewhere in the codebase.

As per coding guidelines, leveraging TypeScript's type system more fully by defining named types improves code clarity and reduces duplication.

Example:

// Type definitions for row styling conversion
export interface IRowDimensions {
  height?: string;
  minHeight?: string;
  maxHeight?: string;
}

export interface ISpacing {
  top?: string;
  right?: string;
  bottom?: string;
  left?: string;
}

export interface IRowStylingBox {
  margin?: ISpacing;
  padding?: ISpacing;
}

export interface IBorderConfig {
  width?: string | number;
  style?: string;
  color?: string;
}

export interface IRowBorderStyle {
  borderType?: string;
  border?: {
    all?: IBorderConfig;
    top?: IBorderConfig;
    right?: IBorderConfig;
    bottom?: IBorderConfig;
    left?: IBorderConfig;
  };
}

// Then use in function signatures
export const convertRowDimensionsToHeight = (rowDimensions?: IRowDimensions): string | undefined => {
  // ...
};
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/dataTable/table/utils.ts around lines
251 to 340, the three conversion functions use inline parameter type literals;
extract and export named interfaces (e.g., IRowDimensions, ISpacing,
IRowStylingBox, IBorderConfig, IRowBorderStyle) at the top of the file and
replace the inline types in the function signatures with these interfaces;
ensure the border width type remains string | number, export the interfaces for
reuse, and update any imports/exports if other modules will consume them.

Loading