IFrame widget improvements

[ehconitin]

No description provided.

[greptile-apps]

Greptile Overview

Greptile Summary

Refactored iframe widget creation and settings flow to provide a better user experience with real-time updates.

Key Changes:

• Renamed CommandMenuPageLayoutIframeConfig to CommandMenuPageLayoutIframeSettings for consistency
• Replaced form-based configuration with instant-update pattern using SidePanelHeader for title editing
• Widget is now created immediately when user selects "iFrame" type, then settings are edited in place
• Split widget size constants: WIDGET_SIZES for general widgets, GRAPH_WIDGET_SIZES for graph-specific sizing
• Added pointer-events disabling on iframe during edit mode to prevent interaction conflicts
• Made useCreatePageLayoutIframeWidget return the created widget for immediate use
• Added minimum size constraints (4x5) for iframe widgets

Issue Found:

• validateUrl is called twice unnecessarily in handleUrlChange (line 79-91 of CommandMenuPageLayoutIframeSettings.tsx)

Confidence Score: 4/5

• This PR is mostly safe to merge with one logical inefficiency that should be addressed
• The refactoring improves UX by switching to real-time updates. The code is well-structured and follows existing patterns. However, the redundant validateUrl call in handleUrlChange could cause issues and should be fixed before merging.
• Pay attention to packages/twenty-front/src/modules/command-menu/pages/page-layout/components/CommandMenuPageLayoutIframeSettings.tsx - fix the double validation call

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-front/src/modules/command-menu/pages/page-layout/components/CommandMenuPageLayoutIframeSettings.tsx	3/5	Refactored iframe configuration to use real-time updates with SidePanelHeader. Contains redundant validateUrl call that should be fixed.
packages/twenty-front/src/modules/command-menu/pages/page-layout/components/CommandMenuPageLayoutWidgetTypeSelect.tsx	5/5	Added iframe widget creation logic when navigating to settings, ensuring widget exists before editing.
packages/twenty-front/src/modules/page-layout/hooks/useCreatePageLayoutIframeWidget.ts	5/5	Updated to use WIDGET_SIZES constant and return created widget for further operations.
packages/twenty-front/src/modules/page-layout/widgets/iframe/components/IframeWidget.tsx	5/5	Added edit mode detection to disable pointer events on iframe, preventing interaction issues during layout editing.

Sequence Diagram

sequenceDiagram
    participant User
    participant CommandMenu
    participant WidgetTypeSelect
    participant IframeSettings
    participant useCreateWidget
    participant useUpdateWidget
    participant IframeWidget
    participant State

    User->>CommandMenu: Click "Add Widget"
    CommandMenu->>WidgetTypeSelect: Show widget type options
    User->>WidgetTypeSelect: Select "iFrame"
    
    alt Widget doesn't exist yet
        WidgetTypeSelect->>useCreateWidget: createPageLayoutIframeWidget("New iFrame", "")
        useCreateWidget->>State: Create widget with default size (6x6)
        useCreateWidget->>State: Add to page layout draft
        useCreateWidget-->>WidgetTypeSelect: Return new widget
        WidgetTypeSelect->>State: Set editing widget ID
    end
    
    WidgetTypeSelect->>IframeSettings: Navigate to settings page
    IframeSettings->>State: Get widget in edit mode
    IframeSettings->>User: Display SidePanelHeader with title editor
    IframeSettings->>User: Display URL input field
    
    alt User changes title
        User->>IframeSettings: Update title
        IframeSettings->>useUpdateWidget: updatePageLayoutWidget(id, {title})
        useUpdateWidget->>State: Update widget title
    end
    
    alt User changes URL
        User->>IframeSettings: Enter URL
        IframeSettings->>IframeSettings: Validate URL
        alt Valid URL
            IframeSettings->>useUpdateWidget: updatePageLayoutWidget(id, {configuration})
            useUpdateWidget->>State: Update widget URL
        else Invalid URL
            IframeSettings->>User: Show validation error
        end
    end
    
    User->>CommandMenu: Close menu
    CommandMenu->>IframeWidget: Render widget with configuration
    
    alt Edit mode active
        IframeWidget->>State: Check isPageLayoutInEditMode
        State-->>IframeWidget: true
        IframeWidget->>User: Display iframe with pointer-events: none
    else Normal mode
        IframeWidget->>State: Check isPageLayoutInEditMode
        State-->>IframeWidget: false
        IframeWidget->>User: Display interactive iframe
    end

Loading

_{17 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: validateUrl is called twice in handleUrlChange. The first call on line 81 sets the error state, then the second call on line 83 is used for the conditional. This is inefficient and could lead to race conditions with the state updates.

Suggested change

	const handleUrlChange = (value: string) => {
	setUrl(value);
	validateUrl(value);
	if (validateUrl(value)) {
	updatePageLayoutWidget(widgetInEditMode.id, {
	configuration: {
	...widgetInEditMode.configuration,
	url: value.trim(),
	},
	});
	}
	};
	const handleUrlChange = (value: string) => {
	setUrl(value);
	if (validateUrl(value)) {
	updatePageLayoutWidget(widgetInEditMode.id, {
	configuration: {
	...widgetInEditMode.configuration,
	url: value.trim(),
	},
	});
	}
	};

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-front/src/modules/command-menu/pages/page-layout/components/CommandMenuPageLayoutIframeSettings.tsx
Line: 79:91

Comment:
**logic:** `validateUrl` is called twice in `handleUrlChange`. The first call on line 81 sets the error state, then the second call on line 83 is used for the conditional. This is inefficient and could lead to race conditions with the state updates.

```suggestion
  const handleUrlChange = (value: string) => {
    setUrl(value);

    if (validateUrl(value)) {
      updatePageLayoutWidget(widgetInEditMode.id, {
        configuration: {
          ...widgetInEditMode.configuration,
          url: value.trim(),
        },
      });
    }
  };
```

How can I resolve this? If you propose a fix, please make it concise.