Skip to content

1315 notebook cell organization#1342

Merged
johbaxter merged 10 commits intodevfrom
1315_notebook_cell_organization
Jun 30, 2025
Merged

1315 notebook cell organization#1342
johbaxter merged 10 commits intodevfrom
1315_notebook_cell_organization

Conversation

@KirthikaKumar-K
Copy link
Copy Markdown
Contributor

Description

Changes Made

How to Test

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

Notes

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

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

1315 notebook cell organization


User description

Description

Changes Made

How to Test

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

Notes


PR Type

Enhancement


Description

  • Group data actions under Data dropdown

  • Update icons and menu styling overrides

  • Simplify button layout, remove dividers, center

  • Replace MoreHoriz with MoreVert icon


Changes walkthrough 📝

Relevant files
Enhancement
NotebookAddCell.tsx
Reorganize and restyle AddCell component                                 

packages/client/src/components/notebook/NotebookAddCell.tsx

  • Remove individual option icons, use grouped options
  • Define DataOptions and attach to data entry
  • Remove StyledDivider, center align button stack
  • Update StyledMenu styling (border-radius, max-height, scroll)
  • Change "others" icon to MoreVert
  • Adjust endIcon logic for dropdown arrows
  • +69/-82 

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

    Undefined Variable

    The options property for the 'transformation' group refers to Transformations, but no such identifier is imported or defined, which will cause runtime errors when opening that menu.

    transformation: {
        display: 'Transformation',
        icon: (
            <svg
                width="18"
                height="18"
                viewBox="0 0 24 24"
                fill="none"
                xmlns="http://www.w3.org/2000/svg"
            >
                <path
                    d="M9.9987 3.33203V0.832031L6.66536 4.16536L9.9987 7.4987V4.9987C12.757 4.9987 14.9987 7.24036 14.9987 9.9987C14.9987 10.8404 14.7904 11.6404 14.4154 12.332L15.632 13.5487C16.282 12.5237 16.6654 11.307 16.6654 9.9987C16.6654 6.31536 13.682 3.33203 9.9987 3.33203ZM9.9987 14.9987C7.24036 14.9987 4.9987 12.757 4.9987 9.9987C4.9987 9.15703 5.20703 8.35703 5.58203 7.66536L4.36536 6.4487C3.71536 7.4737 3.33203 8.69036 3.33203 9.9987C3.33203 13.682 6.31536 16.6654 9.9987 16.6654V19.1654L13.332 15.832L9.9987 12.4987V14.9987Z"
                    fill="#757575"
                />
            </svg>
        ),
        options: Transformations,
    Unused Import

    The Modal component is imported from @semoss/ui but never used in this file; consider removing it to clean up the code.

    import { styled, Button, MenuProps, Menu, Stack, Modal } from '@semoss/ui';
    Inconsistent Quoting

    Backticks are used for some string literals in DataOptions while others use single quotes, leading to inconsistent code style.

    const DataOptions = [
        {
            display: `Filter Data`,
            defaultCellType: 'filter-data',
        },
        {
            display: `UnFilter Data`,
            defaultCellType: `unfilter-data`,
        },
        {
            display: `Custom Import`,
            defaultCellType: 'query-import',
        },
    ];

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Render "others" menu items

    The menu never renders entries for the "others" category. Add a block to map
    OtherOptions when selectedAddCell === 'others' so those items appear in the
    dropdown.

    packages/client/src/components/notebook/NotebookAddCell.tsx [326-347]

    -{selectedAddCell === 'data' && // Ensure we are showing the options for "Data"
    -    DataOptions.map(
    -        ({ display, defaultCellType }, index) => {
    -            return (
    -                <StyledMenuItem
    -                    key={index}
    -                    value={display}
    -                    onClick={() => {
    -                        appendCell(defaultCellType); // Append selected cell
    -                        setAnchorEl(null); // Close the menu
    -                    }}
    -                >
    -                    {display}
    -                </StyledMenuItem>
    -            );
    -        },
    -    )}
    +{selectedAddCell === 'data' &&
    +  DataOptions.map(({ display, defaultCellType }, index) => (
    +    <StyledMenuItem
    +      key={index}
    +      value={display}
    +      onClick={() => {
    +        appendCell(defaultCellType);
    +        setAnchorEl(null);
    +      }}
    +    >
    +      {display}
    +    </StyledMenuItem>
    +  ))}
    +{selectedAddCell === 'transformation' &&
    +  AddCellOptions.transformation.options?.map(({ display, defaultCellType }, index) => (
    +    <StyledMenuItem
    +      key={index}
    +      value={display}
    +      onClick={() => {
    +        appendCell(defaultCellType);
    +        setAnchorEl(null);
    +      }}
    +    >
    +      {display}
    +    </StyledMenuItem>
    +  ))}
    +{selectedAddCell === 'others' &&
    +  OtherOptions.map(({ display, defaultCellType }, index) => (
    +    <StyledMenuItem
    +      key={index}
    +      value={display}
    +      onClick={() => {
    +        appendCell(defaultCellType);
    +        setAnchorEl(null);
    +      }}
    +    >
    +      {display}
    +    </StyledMenuItem>
    +  ))}
     
    -{selectedAddCell === 'transformation' &&
    -    Array.from(
    -        AddCellOptions[selectedAddCell]?.options || [],
    -        ({ display, defaultCellType }, index) => { /* ... */ }
    -    )}
    -
    Suggestion importance[1-10]: 8

    __

    Why: Without a mapping for others, the dropdown never shows the OtherOptions entries, which is a functional bug that needs addressing.

    Medium
    Only show arrows for option items

    The current ternary logic still shows a down arrow for items without options.
    Simplify the condition to only render arrows when value.options is an array, hiding
    the icon otherwise.

    packages/client/src/components/notebook/NotebookAddCell.tsx [297-308]

     endIcon={
    -    add[0] === 'others' ||
    -    add[0] ===
    -        'code' ? null : Array.isArray(
    -      value.options,
    -    ) &&
    -      selectedAddCell === add[0] &&
    -      open ? (
    -        <KeyboardArrowUp />
    -      ) : (
    -        <KeyboardArrowDown />
    -      )
    +  Array.isArray(value.options)
    +    ? selectedAddCell === add[0] && open
    +      ? <KeyboardArrowUp />
    +      : <KeyboardArrowDown />
    +    : null
     }
    Suggestion importance[1-10]: 7

    __

    Why: Simplifying the endIcon logic ensures arrows only appear when value.options is an array and improves readability by removing special-case checks.

    Medium
    General
    Use generic menu mapping

    The menu rendering is duplicated for each category. Replace both blocks with a
    single generic mapping over AddCellOptions[selectedAddCell]?.options to support any
    multi-option entry.

    packages/client/src/components/notebook/NotebookAddCell.tsx [326-347]

    -{selectedAddCell === 'data' && // Ensure we are showing the options for "Data"
    -    DataOptions.map(
    -        ({ display, defaultCellType }, index) => { /* ... */ }
    -    )}
    +{Array.isArray(AddCellOptions[selectedAddCell]?.options) &&
    +  AddCellOptions[selectedAddCell].options.map(
    +    ({ display, defaultCellType }, index) => (
    +      <StyledMenuItem
    +        key={index}
    +        value={display}
    +        onClick={() => {
    +          appendCell(defaultCellType);
    +          setAnchorEl(null);
    +        }}
    +      >
    +        {display}
    +      </StyledMenuItem>
    +    )
    +  )}
     
    -{selectedAddCell === 'transformation' &&
    -    Array.from(
    -        AddCellOptions[selectedAddCell]?.options || [],
    -        ({ display, defaultCellType }, index) => { /* ... */ }
    -    )}
    -
    Suggestion importance[1-10]: 5

    __

    Why: Consolidating duplicated mapping blocks into a single generic loop over AddCellOptions[selectedAddCell]?.options improves maintainability without changing behavior.

    Low

    @johbaxter johbaxter merged commit 083c5eb into dev Jun 30, 2025
    3 checks passed
    @johbaxter johbaxter deleted the 1315_notebook_cell_organization branch June 30, 2025 19:01
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-06-30 *

    Changed

    • Renamed date-difference transformation cell label and fixed typo
    • Reorganized notebook add-cell dropdown: grouped transformation options with search/filter and categories
    • Updated dropdown icons and refined UI styling for add-cell actions

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

    @KirthikaKumar-K KirthikaKumar-K linked an issue Jul 4, 2025 that may be closed by this pull request
    3 tasks
    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 Transformations Menu to Cell Menu Notebook cell organization implementation

    3 participants