Skip to content

Type Property For Button Block and its Migration Function#1387

Merged
neelneelneel merged 10 commits intodevfrom
1357_addtypeproperty_buttonblock
Jul 17, 2025
Merged

Type Property For Button Block and its Migration Function#1387
neelneelneel merged 10 commits intodevfrom
1357_addtypeproperty_buttonblock

Conversation

@KirthikaKumar-K
Copy link
Copy Markdown
Contributor

@KirthikaKumar-K KirthikaKumar-K commented Jun 26, 2025

Description

Type Property for Button Block and its Migration Function

Changes Made

  1. Added Type property to button block
  2. Added Migration function.

How to Test

Steps To Reproduce:

  1. Create/Go to a to Drag and Drop app.
  2. Drag in a button block.
  3. Under block settings we would find the type property

Video

ButtonBlock.mp4

@KirthikaKumar-K KirthikaKumar-K requested a review from a team as a code owner June 26, 2025 06:58
@KirthikaKumar-K KirthikaKumar-K linked an issue Jun 26, 2025 that may be closed by this pull request
3 tasks
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@KirthikaKumar-K KirthikaKumar-K linked an issue Jun 26, 2025 that may be closed by this pull request
1 task
@QodoAI-Agent
Copy link
Copy Markdown

Title

Type Property For Button Block and its Migration Function


User description

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes


PR Type

Enhancement


Description

  • Add type attribute to ButtonBlockDef

  • Expose type in ButtonBlock component

  • Add UI setting for button type

  • Implement state migration for type


Changes walkthrough 📝

Relevant files
Enhancement
ButtonBlock.tsx
Enhance ButtonBlock with `type` property                                 

libs/renderer/src/components/block-defaults/button-block/ButtonBlock.tsx

  • Include type in ButtonBlockDef data
  • Pass type={data?.type} to StyledButton
  • Add console.log for debugging
  • +3/-0     
    migrate__1_0_0_alpha_11__to___1_0_0_alpha_12.ts
    Migration: default button `type` assignment                           

    libs/renderer/src/store/state/migration/migrate__1_0_0_alpha_11__to___1_0_0_alpha_12.ts

  • Add migration for default button type
  • Iterate blocks and set missing type
  • Return updated state with defaults
  • +33/-0   
    Configuration changes
    config.tsx
    Configure button `type` UI setting                                             

    libs/renderer/src/components/block-defaults/button-block/config.tsx

  • Initialize type in default button data
  • Add SelectInputSettings for type property
  • +25/-0   
    MigrationManager.ts
    Bump state version and register migration                               

    libs/renderer/src/store/state/migration/MigrationManager.ts

  • Import new migration module alpha.11→alpha.12
  • Update STATE_VERSION to 1.0.0-alpha.12
  • Register migration in migrations map
  • +4/-2     
    default-menu.ts
    Default menu: add button `type`                                                   

    packages/client/src/components/blocks-workspace/menus/default-menu.ts

    • Set default button type to button
    +1/-0     

    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

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Debug Logging

    A console.log statement remains in production code, which can clutter logs and reveal internal data. Remove or guard this debug output before merging.

    console.log("ButtonBlock", data);
    Migration Side Effects

    The migration directly mutates the input state and uses hasOwnProperty unsafely. Consider cloning objects or using a safer pattern to avoid unintended state mutations and prototype pollution.

    for (const key in blocks) {
        // Check if the current property is a direct property of blocks
        if (blocks.hasOwnProperty(key)) {
            const block = blocks[key]; // Get the block associated with the current key
    
            // Check if the block is a button widget
            if (block.widget === "button") {
                // Set the type to "button" if it's not already defined in block's data
                if (!block.data.type) {
                    block.data.type = "button";
                }
            }
            blocks[key] = block;
        }
    }

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    QodoAI-Agent commented Jun 26, 2025

    PR Code Suggestions ✨

    Latest suggestions up to ab4b37b

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Provide default button type

    Ensure the type prop is never undefined by providing a fallback of "button". This
    prevents unintended default form submissions when data.type is missing.

    libs/renderer/src/components/block-defaults/button-block/ButtonBlock.tsx [75]

    -type={data?.type}
    +type={data.type || 'button'}
    Suggestion importance[1-10]: 6

    __

    Why: The fallback type={data.type || 'button'} prevents unintended form submissions when data.type is undefined, improving reliability with moderate impact.

    Low

    Previous suggestions

    Suggestions up to commit 41423b7
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Guard migration on valid data

    Add a guard to skip entries without a valid data object to prevent runtime errors on
    malformed or unexpected block entries. This ensures only blocks with a data property
    get processed.

    libs/renderer/src/store/state/migration/migrate__1_0_0_alpha_11__to___1_0_0_alpha_12.ts [12-18]

     for (const [key, block] of Object.entries(blocks)) {
    +    if (!block?.data || typeof block.data !== 'object') {
    +        newBlocks[key] = block;
    +        continue;
    +    }
         // Clone the block to avoid mutation
         const newBlock = { ...block, data: { ...block.data } };
         if (newBlock.widget === "button" && !newBlock.data.type) {
             newBlock.data.type = "button";
         }
         newBlocks[key] = newBlock;
     }
    Suggestion importance[1-10]: 7

    __

    Why: Adding a check for block?.data prevents runtime errors on malformed entries, making the migration more robust without breaking existing logic.

    Medium
    Add default button type

    Provide a default fallback for the button type to avoid unintended form submissions
    when data.type is undefined. This ensures the button never defaults to HTML's
    implicit "submit" type.

    libs/renderer/src/components/block-defaults/button-block/ButtonBlock.tsx [75]

    -type={data?.type}
    +type={data?.type ?? 'button'}
    Suggestion importance[1-10]: 6

    __

    Why: The fallback data?.type ?? 'button' prevents unintended form submissions when data.type is undefined, improving component safety with minimal impact.

    Low
    General
    Reorder dropdown options

    Reorder the type options to list "button" first, matching the default data and
    reducing confusion in the dropdown.

    libs/renderer/src/components/block-defaults/button-block/config.tsx [77-90]

     options={[
    +    {
    +        value: "button",
    +        display: "Button",
    +    },
         {
             value: "submit",
             display: "Submit",
    -    },
    -    {
    -        value: "button",
    -        display: "Button",
         },
         {
             value: "reset",
             display: "Reset",
         },
     ]}
    Suggestion importance[1-10]: 3

    __

    Why: Reordering options to match the default reduces minor confusion for users but does not affect functionality.

    Low
    Suggestions up to commit e3390b8
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add default button type

    Provide an explicit fallback to "button" to avoid unintended "submit" behavior when
    data.type is undefined. This ensures consistent default action.

    libs/renderer/src/components/block-defaults/button-block/ButtonBlock.tsx [76]

    -type={data?.type}
    +type={data.type ?? "button"}
    Suggestion importance[1-10]: 7

    __

    Why: Providing a fallback of "button" ensures the HTML button does not default to "submit" unexpectedly for undefined data.type.

    Medium
    Guard missing block data

    Guard against missing data to avoid runtime exceptions. Ensure block.data exists
    before reading or writing the type property.

    libs/renderer/src/store/state/migration/migrate__1_0_0_alpha_11__to___1_0_0_alpha_12.ts [20-22]

    -if (!block.data.type) {
    -    block.data.type = "button";
    +if (block.data) {
    +    if (!block.data.type) {
    +        block.data.type = "button";
    +    }
     }
    Suggestion importance[1-10]: 4

    __

    Why: Adding a check for block.data prevents potential runtime errors but is a minor defensive improvement.

    Low
    General
    Remove debug logging

    Remove or guard this debug logging to prevent noisy output in production. If you
    need runtime diagnostics, consider using a debug logger or conditional log based on
    environment.

    libs/renderer/src/components/block-defaults/button-block/ButtonBlock.tsx [67]

    -console.log("ButtonBlock", data);
    +// Remove debug log or replace with a debug-level logger if needed
    Suggestion importance[1-10]: 5

    __

    Why: Removing the stray console.log prevents noisy output in production and improves code cleanliness.

    Low

    @KirthikaKumar-K KirthikaKumar-K linked an issue Jul 4, 2025 that may be closed by this pull request
    1 task
    @j-adam-connor
    Copy link
    Copy Markdown
    Contributor

    j-adam-connor commented Jul 7, 2025

    Hello! Just a couple of things I'd point out before beginning review for this:

    • The PR description is incomplete.
    • The PR agent identifies some potential issues that may need your attention here

    Thank you!

    @KirthikaKumar-K
    Copy link
    Copy Markdown
    Contributor Author

    Hello! Just a couple of things I'd point out before beginning review for this:

    • The PR description is incomplete.
    • The PR agent identifies some potential issues that may need your attention here

    Thank you!

    Hi @j-adam-connor ,

    Thank you !


    // Return a new state object with updated blocks
    return { ...state, blocks: newBlocks };
    }
    Copy link
    Copy Markdown
    Contributor

    @j-adam-connor j-adam-connor Jul 8, 2025

    Choose a reason for hiding this comment

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

    This curly brace should be indented with a trailing comma. Double check your formatter is configured correctly.

    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.

    It looks like we have an established pattern that's a bit different from this one where we destructure the state into newState; e.g., const newState = { ... state }; and then we ultimately return newState

    Copy link
    Copy Markdown
    Contributor Author

    Choose a reason for hiding this comment

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

    Hi @j-adam-connor, I've made the updates based on your comments. You can review the changes here

    @j-adam-connor
    Copy link
    Copy Markdown
    Contributor

    Hey, looks like there are some conflicts. Please resolve, and we'll get this merged. Thanks!

    @neelneelneel neelneelneel merged commit b84d6a7 into dev Jul 17, 2025
    3 checks passed
    @neelneelneel neelneelneel deleted the 1357_addtypeproperty_buttonblock branch July 17, 2025 14:33
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-07-17 *

    Added

    • Support for a new type property on button blocks.
    • Migration to backfill existing button blocks with a default type.

    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.

    Add Type Property to Button Block

    4 participants