Skip to content

402 task update grid block#966

Merged
johbaxter merged 11 commits intodevfrom
402-task-update-grid-block
May 30, 2025
Merged

402 task update grid block#966
johbaxter merged 11 commits intodevfrom
402-task-update-grid-block

Conversation

@Shubham009-prog
Copy link
Copy Markdown
Contributor

Description

Replaced normal table with Data Grid Block (MUI)

Added features:

  • Title Styling
  • Header Styling
  • Color By value
  • Column Resizing
  • Cell Styling
  • Row Spanning
  • Text Wrapping
  • Pagination
  • Resizing
  • Click event
  • Also added color picker as shown in figma

@Shubham009-prog Shubham009-prog self-assigned this Apr 23, 2025
@Shubham009-prog Shubham009-prog requested a review from a team as a code owner April 23, 2025 11:48
@Shubham009-prog Shubham009-prog linked an issue Apr 23, 2025 that may be closed by this pull request
12 tasks
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

402 task update grid block


User description

Description

Replaced normal table with Data Grid Block (MUI)

Added features:

  • Title Styling
  • Header Styling
  • Color By value
  • Column Resizing
  • Cell Styling
  • Row Spanning
  • Text Wrapping
  • Pagination
  • Resizing
  • Click event
  • Also added color picker as shown in figma

PR Type

Enhancement


Description

  • Introduce DataGrid variation for grid block

  • Add header, cell, title styling operations

  • Support wrap text, row spanning, color-by-value rules

  • Implement tabbed settings menu and config update


Changes walkthrough 📝

Relevant files
Enhancement
13 files
GridBlock.tsx
Enhance original grid block with styling options                 
+158/-1 
GridBlockDuplicate.tsx
New DataGrid‑based grid block component                                   
+442/-0 
GridBlockMenu.tsx
Introduce tabbed settings menu for grid block                       
+111/-0 
GridBlockTool.tsx
Assemble grid styling tools in side panel                               
+330/-0 
CellStyling.tsx
New cell styling settings component                                           
+272/-0 
ChartTitle.tsx
Add chart title settings operation                                             
+226/-0 
ColorByValue.tsx
Implement color‑by‑value rule editor                                         
+426/-0 
GridResizeSettings.tsx
Create generic size settings control                                         
+206/-0 
HeaderStyling.tsx
Build header styling settings component                                   
+273/-0 
RowSpanning.tsx
Add row spanning toggle operation                                               
+81/-0   
WrapTextSettings.tsx
Provide text wrap settings per column                                       
+199/-0 
ColorPickerSettingsNew.tsx
New advanced color picker component                                           
+209/-0 
ColorPickerWithSwatch.tsx
Add simple color swatch input                                                       
+56/-0   
Debugging
1 files
GridBlockColumnSettings.tsx
Add debug logs for header sync functionality                         
+3/-0     
Configuration changes
2 files
config.tsx
Switch to new variation and update default config               
+23/-55 
default-menu.ts
Update default grid block JSON defaults                                   
+11/-0   
Dependencies
1 files
package.json
Update grid dependencies and add data‑grid                             
+13/-12 
Additional files
1 files
pnpm-lock.yaml [link]   

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Debug Logs

    Console.log statements remain in production code and should be removed or replaced with proper logging or debugging tools.

    console.log(data, "DATA");
    console.log(frame, "Frame");
    Initialization Error

    Constants headerSettings, cellSettings, and wrapTextSettings are referenced before they are defined, causing a runtime ReferenceError.

    const columns = data.columns.map((col) => ({
        field: col.name,
        headerName: col.name,
        sortable: false,
        renderHeader: () => (
            <div
                style={{
                    // Apply style if the column is selected
                    backgroundColor: headerSettings.selectedColumn.includes(
    State Mismatch

    The handler uses selectedColumns instead of the declared selectedColumn property, so selected columns won't update correctly.

    const handleColumnChange = (_, selected: GridBlockColumn[]) => {
        const newSelected = selected.map((col) => col.name);
        const newOption = {
            ...data.option,
            cellBackgroundSettings: {
                ...gridStyle,
                selectedColumn: newSelected,
            },
        };

    @QodoAI-Agent
    Copy link
    Copy Markdown

    QodoAI-Agent commented Apr 23, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 50409fe

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Hoist settings above columns

    Move the declarations of headerSettings, cellSettings, titleSettings, and
    wrapTextSettings above the columns mapping so they are defined before use and avoid
    runtime reference errors.

    libs/renderer/src/components/block-defaults/grid-block/GridBlockDuplicate.tsx [203-207]

    +const headerSettings = { /* ... */ };
    +const cellSettings   = { /* ... */ };
    +const titleSettings  = { /* ... */ };
    +const wrapTextSettings = { /* ... */ };
    +
     const columns = data.columns.map((col) => ({
       field: col.name,
       headerName: col.name,
       renderHeader: () => (
         <div style={{
           backgroundColor: headerSettings.selectedColumn.includes(col.name)
             ? headerSettings.backgroundColor
             : "inherit",
     ...
    Suggestion importance[1-10]: 9

    __

    Why: Moves the headerSettings, cellSettings, titleSettings, and wrapTextSettings declarations before the columns mapping to avoid undefined references and runtime errors.

    High
    Fix selectedColumn state typo

    Correct the property name from selectedColumns to selectedColumn so the state
    matches the CellBackgroundSettings interface and updates correctly.

    libs/renderer/src/components/block-defaults/grid-block/operations/CellStyling.tsx [78-81]

     setGridStyle((prev) => ({
         ...prev,
    -    selectedColumns: newSelected,
    +    selectedColumn: newSelected,
     }));
    Suggestion importance[1-10]: 8

    __

    Why: Corrects the property name from selectedColumns to selectedColumn to match the CellBackgroundSettings interface and ensure column selection works.

    Medium
    Add timeout cleanup on unmount

    Clear any pending timeouts when the component unmounts to avoid memory leaks and
    unintended state updates. Use a cleanup effect to clear both timeoutRef and
    optiontimeoutRef.

    libs/renderer/src/components/block-settings/shared/ColorPickerSettingsNew.tsx [66-67]

    -const timeoutRef = useRef<ReturnType<typeof setTimeout>>(null);
    -const optiontimeoutRef = useRef<ReturnType<typeof setTimeout>>(null);
    +const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
    +const optiontimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
     
    +useEffect(() => {
    +  return () => {
    +    if (timeoutRef.current) clearTimeout(timeoutRef.current);
    +    if (optiontimeoutRef.current) clearTimeout(optiontimeoutRef.current);
    +  };
    +}, []);
    +
    Suggestion importance[1-10]: 7

    __

    Why: Clearing timeoutRef and optiontimeoutRef in a cleanup effect prevents potential memory leaks and stray state updates.

    Medium
    General
    Initialize value with computed data

    Initialize the local value state using the current computedValue to avoid showing
    "null" in the input and prevent controlled/uncontrolled warnings. This ensures the
    input reflects the actual data immediately.

    libs/renderer/src/components/block-settings/shared/ColorPickerSettingsNew.tsx [65]

    -const [value, setValue] = useState<string | null>(null);
    +const [value, setValue] = useState<string>(computedValue);
    Suggestion importance[1-10]: 6

    __

    Why: Initializing value with computedValue prevents a null input and controlled/uncontrolled warnings, improving UX.

    Low
    Remove debug console logs

    Remove stray console.log statements used for debugging, as they clutter the console
    and may expose internal data in production.

    libs/renderer/src/components/block-defaults/grid-block/GridBlock.tsx [198-199]

    -console.log(data, "DATA");
    -console.log(frame, "Frame");
    +(Remove these two lines)
    Suggestion importance[1-10]: 5

    __

    Why: Removing stray console.log statements cleans up the console output and prevents exposing internal data in production.

    Low
    Remove unused color state

    The color state is never used to render UI; remove this redundant state and its
    setters to simplify the component and avoid confusing duplication of color value.

    libs/renderer/src/components/block-settings/shared/ColorPickerSettingsNew.tsx [63]

    -const [color, setColor] = useState(colorValue);
    +// Removed redundant color state
    Suggestion importance[1-10]: 5

    __

    Why: The color state and its setter are never used in rendering, so removing them simplifies the component.

    Low
    Use flex-start alignment

    Use the standard flex alignment value "flex-start" instead of "start" to ensure
    cross-browser consistency.

    libs/renderer/src/components/block-defaults/grid-block/GridBlock.tsx [35-39]

     const StyledTitle = styled("div")(() => ({
         display: "flex",
    -
    -    justifyContent: "start",
    +    justifyContent: "flex-start",
     }));
    Suggestion importance[1-10]: 4

    __

    Why: Replaces the nonstandard "start" value with "flex-start" for justifyContent to ensure consistent flexbox behavior across browsers.

    Low
    Add prop type for styled component

    Explicitly type the color prop for the styled component to ensure TypeScript infers
    it correctly and avoid missing property errors at compile time.

    libs/renderer/src/components/block-settings/shared/ColorPickerSettingsNew.tsx [44-52]

    -const StyledSpanSection = styled("span")(({ color }) => ({
    +const StyledSpanSection = styled("span")<{ color: string }>(({ color }) => ({
         backgroundColor: color,
         width: "23px",
         height: "23px",
         borderRadius: "20%",
         display: "block",
         border: "1px solid #000",
         marginRight: "12px",
     }));
    Suggestion importance[1-10]: 4

    __

    Why: Explicitly typing the color prop makes TypeScript checks stricter, avoiding potential missing property errors.

    Low

    Previous suggestions

    Suggestions up to commit b511358
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Initialize settings before columns

    Move the definitions of headerSettings, cellSettings, wrapTextSettings, and
    colorRules above the columns mapping so they are initialized before use.

    libs/renderer/src/components/block-defaults/grid-block/GridBlockDuplicate.tsx [205-213]

    +const headerSettings = { /* ... */ };
    +const cellSettings = { /* ... */ };
    +const wrapTextSettings = { /* ... */ };
    +const colorRules: ColorRule[] = data.option?.colorByValue || [];
    +
     const columns = data.columns.map((col) => ({
       field: col.name,
       headerName: col.name,
       renderHeader: () => (
         <div style={{
           backgroundColor: headerSettings.selectedColumn.includes(col.name)
             ? headerSettings.backgroundColor
             : "inherit",
           /* ... */
         }}>{col.name}</div>
       ),
       /* ... */
     }));
    Suggestion importance[1-10]: 9

    __

    Why: columns mapping references headerSettings (and related settings) before they are declared, leading to a ReferenceError at runtime.

    High
    Fix state property naming

    Use the correct selectedColumn property instead of selectedColumns to match the
    state shape.

    libs/renderer/src/components/block-defaults/grid-block/operations/HeaderStyling.tsx [79-82]

     setGridStyle((prev) => ({
         ...prev,
    -    selectedColumns: newSelected,
    +    selectedColumn: newSelected,
     }));
    Suggestion importance[1-10]: 9

    __

    Why: The handler mistakenly uses selectedColumns instead of the defined selectedColumn field, causing the UI state to never update correctly; renaming it is essential for functionality.

    High
    Fix state property name

    Correct the property name from selectedColumns to selectedColumn to match the state
    shape.

    libs/renderer/src/components/block-defaults/grid-block/operations/CellStyling.tsx [80-83]

     setGridStyle((prev) => ({
         ...prev,
    -    selectedColumns: newSelected,
    +    selectedColumn: newSelected,
     }));
    Suggestion importance[1-10]: 8

    __

    Why: The property selectedColumns is incorrect according to the CellBackgroundSettings interface and should be selectedColumn to properly update state.

    Medium
    Correct wrapTextSettings key

    Replace selectedColumns with selectedColumn in the state update to align with the
    initial state definition.

    libs/renderer/src/components/block-defaults/grid-block/operations/WrapTextSettings.tsx [75-78]

     setWrapTextSettings((prev) => ({
         ...prev,
    -    selectedColumns: newSelected,
    +    selectedColumn: newSelected,
     }));
    Suggestion importance[1-10]: 8

    __

    Why: The code updates a non‑existent selectedColumns property on the wrapTextSettings state, breaking the selection logic; correcting it to selectedColumn aligns with the initial state definition.

    Medium
    General
    Remove debug logging

    Remove or guard debug console.log calls to prevent leaking runtime data in
    production.

    libs/renderer/src/components/block-defaults/grid-block/GridBlock.tsx [198-199]

    -console.log(data, "DATA");
    -console.log(frame, "Frame");
    +// Debug logs removed
    Suggestion importance[1-10]: 3

    __

    Why: Debug console.log calls should be removed to avoid leaking internal data in production, but they are minor style cleanup.

    Low

    @ehynd
    Copy link
    Copy Markdown
    Contributor

    ehynd commented May 14, 2025

    @Shubham009-prog a few changes since this pr - could you please merge in dev and retest / fix as needed please?

    @johbaxter johbaxter merged commit 8f602a4 into dev May 30, 2025
    3 checks passed
    @johbaxter johbaxter deleted the 402-task-update-grid-block branch May 30, 2025 15:36
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-05-30 *

    • Swapped default table for MUI Data Grid block.
    • Added styling controls (title, header, cell), color-by-value, text wrapping, row spanning, resizing, pagination, click events and color picker.

    to commit the new content to the CHANGELOG.md file, please type:
    '/update_changelog --pr_update_changelog.push_changelog_changes=true'

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    [TASK] Update Grid Block

    4 participants