Skip to content

Keyboard shortcut to delete blocks#1308

Merged
johbaxter merged 3 commits intodevfrom
feat/MultipleBlockDeletion
Jun 19, 2025
Merged

Keyboard shortcut to delete blocks#1308
johbaxter merged 3 commits intodevfrom
feat/MultipleBlockDeletion

Conversation

@NaveenRamasamy028
Copy link
Copy Markdown
Contributor

User will be able to delete single block or multiple block using keyboard shortcut ctrl + shift + x

@NaveenRamasamy028 NaveenRamasamy028 requested a review from a team as a code owner June 17, 2025 12:29
@NaveenRamasamy028 NaveenRamasamy028 linked an issue Jun 17, 2025 that may be closed by this pull request
3 tasks
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

Keyboard shortcut to delete blocks


User description

User will be able to delete single block or multiple block using keyboard shortcut ctrl + shift + x


PR Type

Enhancement


Description

  • Add Ctrl/Cmd+Shift+X keyboard shortcut for deletion

  • Delete selected blocks, single or multiple

  • Exclude blocks with page ID from deletion

  • Prevent default behavior and event propagation


Changes walkthrough 📝

Relevant files
Enhancement
Screen.tsx
Add keyboard shortcut for block deletion                                 

packages/client/src/components/designer/Screen.tsx

  • Imported ActionMessages from renderer
  • Added keydown listener for Ctrl/Cmd+Shift+X
  • Handled single and multiple block deletion
  • Excluded blocks containing page in ID
  • +48/-1   

    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: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Hook Invocation

    The useBlocks hook is imported but never called, yet state.dispatch is used in the new effect. This will cause a runtime error because state is undefined.

    import { ActionMessages, useBlocks } from '@semoss/renderer';
    Effect Dependency Mismatch

    The keyboard shortcut effect lists only designer in its dependencies but also uses state.dispatch. Missing dependencies can lead to stale closures or unintended behavior.

    useEffect(() => {
        const handleKeyDown = (event: KeyboardEvent) => {
            if (
                (event.ctrlKey || event.metaKey) &&
                event.shiftKey &&
                (event.key === 'x' || event.key === 'X')
            ) {
                if (designer.selected) {
                    // Prevent deletion if id contains 'page'
                    if (designer.selected.includes('page')) {
                        return;
                    }
                    // Delete the selected block
                    state.dispatch({
                        message: ActionMessages.REMOVE_BLOCK,
                        payload: {
                            id: designer.selected,
                            keep: false,
                        },
                    });
                    designer.setSelected('');
                } else if (designer.selectedBlocks.length > 0) {
                    // Delete all multiselected blocks
                    designer.selectedBlocks.forEach((id: string) => {
                        if (!id.includes('page')) {
                            state.dispatch({
                                message: ActionMessages.REMOVE_BLOCK,
                                payload: {
                                    id: id,
                                    keep: false,
                                },
                            });
                        }
                    });
                    designer.addBlockToSelected('clear');
                }
                event.preventDefault();
                event.stopPropagation();
            }
        };
    
        document.addEventListener('keydown', handleKeyDown);
        return () => {
            document.removeEventListener('keydown', handleKeyDown);
        };
    }, [designer]);

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Clear multi-selection correctly

    Calling addBlockToSelected('clear') misuses the API and adds a block with id
    "clear". Instead, explicitly clear the multi-selection with the proper method or
    setter.

    packages/client/src/components/designer/Screen.tsx [309]

    -designer.addBlockToSelected('clear');
    +designer.setSelectedBlocks([]);
    Suggestion importance[1-10]: 8

    __

    Why: Calling addBlockToSelected('clear') likely introduces a phantom block, so explicitly clearing with setSelectedBlocks([]) fixes a real logic bug.

    Medium
    General
    Refine page ID guard

    Using includes('page') may block IDs that simply contain the string "page". Use a
    more precise check such as startsWith('page-') (or your actual prefix) to only guard
    true page IDs.

    packages/client/src/components/designer/Screen.tsx [284]

    -if (designer.selected.includes('page')) {
    +if (designer.selected.startsWith('page-')) {
    Suggestion importance[1-10]: 6

    __

    Why: Checking for 'page' via .includes may block unintended IDs, so using a precise prefix match prevents false positives.

    Low
    Simplify effect dependencies

    Use an empty dependency array for the effect to avoid reattaching the keydown
    listener on every render and prevent stale closures. The MobX designer object
    reference is stable, so the handler will still see updated properties.

    packages/client/src/components/designer/Screen.tsx [320]

    -}, [designer]);
    +}, []);
    Suggestion importance[1-10]: 5

    __

    Why: Changing the effect to run only once improves performance by avoiding redundant listener reattachments without breaking the handler since the observable designer reference remains stable.

    Low
    Use lowercase key check

    Normalize the key comparison by using event.key.toLowerCase() for a single
    case-insensitive check, reducing duplication and potential mistakes.

    packages/client/src/components/designer/Screen.tsx [277-281]

     if (
         (event.ctrlKey || event.metaKey) &&
         event.shiftKey &&
    -    (event.key === 'x' || event.key === 'X')
    +    event.key.toLowerCase() === 'x'
     ) {
    Suggestion importance[1-10]: 5

    __

    Why: Normalizing event.key to lowercase simplifies the conditional and reduces duplication without affecting functionality.

    Low

    @RNubla RNubla self-requested a review June 19, 2025 17:40
    @johbaxter johbaxter merged commit 561afb4 into dev Jun 19, 2025
    3 checks passed
    @johbaxter johbaxter deleted the feat/MultipleBlockDeletion branch June 19, 2025 17:59
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-06-19

    Added

    • Keyboard shortcut (Ctrl + Shift + X) to delete single or multiple blocks.

    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.

    [Enhancement] Keyboard shortcut to delete blocks

    3 participants