Skip to content

feat(client/renderer): added open event to the listeners to perform o…#1323

Merged
johbaxter merged 4 commits intodevfrom
1298-Create-onClick-action-for-open-page
Jun 19, 2025
Merged

feat(client/renderer): added open event to the listeners to perform o…#1323
johbaxter merged 4 commits intodevfrom
1298-Create-onClick-action-for-open-page

Conversation

@Paulson-Robert
Copy link
Copy Markdown
Contributor

…pen action

Description

Changes Made

How to Test

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

Notes

@Paulson-Robert Paulson-Robert requested a review from johbaxter June 19, 2025 09:29
@Paulson-Robert Paulson-Robert self-assigned this Jun 19, 2025
@Paulson-Robert Paulson-Robert requested a review from a team as a code owner June 19, 2025 09:29
@Paulson-Robert Paulson-Robert linked an issue Jun 19, 2025 that may be closed by this pull request
5 tasks
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(client/renderer): added open event to the listeners to perform o…


User description

…pen action

Description

Changes Made

How to Test

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

Notes


PR Type

Enhancement


Description

  • Introduce Open action event support

  • Add UI to select open destination type

  • Extend state to dispatch open events

  • Navigate workspace tabs on open event


Changes walkthrough 📝

Relevant files
Enhancement
6 files
ListenerActionOverlay.tsx
Add open action controls in listener overlay                         
+125/-1 
ListenerSettings.tsx
Render open destinations as clickable links                           
+24/-3   
state.actions.ts
Define DISPATCH_OPEN_EVENT action type                                     
+10/-0   
state.store.ts
Implement dispatchOpenEvent logic in store                             
+32/-0   
state.types.ts
Include open event in ListenerActions type                             
+2/-2     
Workspace.tsx
Handle OPEN_EVENT to navigate or add tabs                               
+80/-1   
Configuration changes
1 files
state.constants.ts
Map DISPATCH_OPEN_EVENT to display label                                 
+1/-0     
Bug fix
1 files
BlocksWorkspaceActions.tsx
Remove page IDs from URL before preview                                   
+13/-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 /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
    🔒 Security concerns

    Open Redirect:
    The dispatchOpenEvent handler sets window.location.href based on unvalidated payload, which could be exploited to redirect users to malicious domains. Consider validating or restricting URLs.

    ⚡ Recommended focus areas for review

    Undefined Variable

    The condition uses lis.message but lis is not defined, which will cause a runtime error. It should reference the correct listener variable.

    : lis.message === ActionMessages.DISPATCH_OPEN_EVENT
    Typo

    The variable distinationType is misspelled and should be destinationType to match the form field and prevent logical errors.

    const distinationType = watch("payload.destinationType");
    
    const pages = useMemo(() => {
        return state.getAllBlocksOfType("page").map((page) => page.id);
    }, [distinationType]);
    Open Redirect

    Using window.location.href directly from payload without validation can lead to open redirect vulnerabilities. Consider validating or restricting allowed destinations.

    private dispatchOpenEvent = (destinationType: string, destination: string): void => {
        const event = new CustomEvent("OPEN_EVENT", { detail: {destinationType, destination}});
    
        if(this.mode === "interactive"){
            if(destinationType === "App Page"){
                const currentUrl = window.location.href;
                const pageIds = this.getAllBlocksOfType('page').map((page) => page.id);
                const pageIdInUrl = pageIds.find((id) => currentUrl.includes(id));
                let newUrl;
                if (pageIdInUrl) 
                    newUrl = currentUrl.replace(pageIdInUrl, destination);
                else
                    newUrl = currentUrl + `/${destination}`
    
                window.location.href = newUrl
            } else if(destinationType === "External"){
                window.location.href = destination
            }
        }
    
        // dispatch the event to the window
        window.dispatchEvent(event);
    }

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Correct undefined variable reference

    The reference lis is undefined in this context. Replace it with the correct
    expression, such as listeners[listener].order[actionIdx], to properly check the
    previous action's message.

    libs/renderer/src/components/block-settings/ListenerActionOverlay.tsx [106-115]

    -: lis.message === ActionMessages.DISPATCH_OPEN_EVENT
    +: listeners[listener].order[actionIdx].message === ActionMessages.DISPATCH_OPEN_EVENT
     ? {
         defaultValues: {
             message: ActionMessages.DISPATCH_OPEN_EVENT,
             payload: {
                 destinationType: "",
                 destination: "",
             },
         },
       }
    Suggestion importance[1-10]: 9

    __

    Why: Replacing lis with the correct listeners[listener].order[actionIdx] prevents a runtime error and restores the intended branch logic.

    High
    Ensure selectedNode is assigned

    After adding a new tab, selectedNode remains null and getId() will throw. Retrieve
    the newly created node (e.g., via layoutModel.getNodeById(destination)) or assign it
    to selectedNode before calling getId().

    packages/client/src/components/workspace/Workspace.tsx [195-214]

     if (!selectedNode) {
         layoutModel.doAction(
             Actions.addNode(
                 {
                     type: 'tab',
                     name: name,
                     component: 'designer',
                     config: { id: destination },
                     enableClose: true,
                 },
                 addId,
                 DockLocation.CENTER,
                 -1,
                 true,
             ),
         );
    +    selectedNode = layoutModel.getNodeById(destination) as TabNode;
     }
     
     const selectedNodeId = selectedNode.getId();
     layoutModel.doAction(Actions.selectTab(selectedNodeId));
    Suggestion importance[1-10]: 9

    __

    Why: Assigning the newly created selectedNode before calling getId() prevents a null dereference and ensures the new tab is selectable.

    High
    Fix typo in variable name

    There's a typo in the variable name distinationType used for watching the payload.
    Rename it consistently to destinationType to ensure the form control logic executes
    correctly.

    libs/renderer/src/components/block-settings/ListenerActionOverlay.tsx [136-140]

    -const distinationType = watch("payload.destinationType");
    +const destinationType = watch("payload.destinationType");
     ...
     const pages = useMemo(() => {
         return state.getAllBlocksOfType("page").map((page) => page.id);
    -}, [distinationType]);
    +}, [destinationType]);
    Suggestion importance[1-10]: 4

    __

    Why: The typo in distinationType only affects readability and naming consistency and does not break functionality.

    Low
    General
    Dispatch event before navigation

    Navigating via window.location.href will unload the page before the event dispatch
    occurs. Move window.dispatchEvent(event) before any window.location.href assignments
    to ensure the custom event fires.

    libs/renderer/src/store/state/state.store.ts [1709-1731]

     private dispatchOpenEvent = (destinationType: string, destination: string): void => {
         const event = new CustomEvent("OPEN_EVENT", { detail: {destinationType, destination}});
    +    // dispatch the event before navigation
    +    window.dispatchEvent(event);
     
    -    if(this.mode === "interactive"){
    -        if(destinationType === "App Page"){
    +    if (this.mode === "interactive") {
    +        if (destinationType === "App Page") {
                 // ...
    -            window.location.href = newUrl
    -        } else if(destinationType === "External"){
    -            window.location.href = destination
    +            window.location.href = newUrl;
    +        } else if (destinationType === "External") {
    +            window.location.href = destination;
             }
         }
    -
    -    // dispatch the event to the window
    -    window.dispatchEvent(event);
     }
    Suggestion importance[1-10]: 6

    __

    Why: Moving window.dispatchEvent(event) before window.location.href ensures the custom event is fired before the page unloads during navigation.

    Low

    Copy link
    Copy Markdown
    Contributor

    @RNubla RNubla left a comment

    Choose a reason for hiding this comment

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

    Reviewed and tested changes. Some minor changes suggestion should be made for better readability.

    @@ -102,6 +103,16 @@ export const ListenerActionOverlay = observer(
    },
    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.

    can this whole section be refactored for better readability like

    const { control, handleSubmit, reset, watch, setValue } = useForm<ListenerActionForm>({
                defaultValues: (() => {
                    if (lis) {
                        if (lis.message === ActionMessages.RUN_CELL) {
                            return {
                                message: ActionMessages.RUN_CELL,
                                payload: { queryId: "", cellId: "" },
                            };
                        }if (
                            lis.message === ActionMessages.DISPATCH_OPEN_EVENT
                        ) {
                            return {
                                message: ActionMessages.DISPATCH_OPEN_EVENT,
                                payload: { destinationType: "", destination: "" },
                            };
                        }
                    }
                    return {
                        message: ActionMessages.RUN_QUERY,
                        payload: { queryId: "" },
                    };
                })(),
            });


    // the type
    const message = watch("message");
    const distinationType = watch("payload.destinationType");
    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.

    please be mindful when using watch(), this will re-render at the root level, which can cause performance issues. As a rule of thumb, use the useWatch hook as much possible

    {content}
    {isLink(content) ? (
    <Link
    target="_blank"
    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.

    add rel="noopener noreferrer" attribute to <Link/>

    @johbaxter johbaxter merged commit f05bbfa into dev Jun 19, 2025
    3 checks passed
    @johbaxter johbaxter deleted the 1298-Create-onClick-action-for-open-page branch June 19, 2025 22:50
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-06-19 *

    Added

    • Open event listener and “Navigate” action for internal/external navigation
    • dispatchOpenEvent in state store with OPEN_EVENT handling in Workspace, plus URL cleanup before preview

    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.

    Create onClick action for open page

    4 participants