Skip to content

Enhancement of Data tab Styling#967

Merged
johbaxter merged 31 commits intodevfrom
feat/enhance-data-tab
May 19, 2025
Merged

Enhancement of Data tab Styling#967
johbaxter merged 31 commits intodevfrom
feat/enhance-data-tab

Conversation

@Gowrishankar-Palanisamy
Copy link
Copy Markdown
Contributor

This PR contains the following changes

-->Updated the data tab styling in all existing charts and ideally create a way for this to be replicated in future charts as well.
-->Updated the current charts in dev branch include: bar, line, scatter, pie, stack.

@Gowrishankar-Palanisamy Gowrishankar-Palanisamy requested a review from a team as a code owner April 23, 2025 11:56
@Gowrishankar-Palanisamy Gowrishankar-Palanisamy linked an issue Apr 23, 2025 that may be closed by this pull request
2 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

Enhancement of Data tab Styling


User description

This PR contains the following changes

-->Updated the data tab styling in all existing charts and ideally create a way for this to be replicated in future charts as well.
-->Updated the current charts in dev branch include: bar, line, scatter, pie, stack.

PR Type

Enhancement, Bug fix


Description

• Unified FrameOperations for chart data tab
• Introduced VisualMap for visual selection
• Added DataTabStyling UI with drag‑drop
• Refactored VisualizationBlockMenu for variants
• Bug fix: Pie selector and Gantt checks


Changes walkthrough 📝

Relevant files
Enhancement
7 files
FrameOperations.tsx
Add unified FrameOperations component for data tab             
+1072/-0
DataTabStyling.tsx
Create DataTabStyling UI for configuration panel                 
+431/-0 
VisualizationBlockMenu.tsx
Refactor menu to use generic FrameOperations                         
+93/-25 
VisualMap.tsx
Add VisualMap component for selecting visuals                       
+163/-0 
Constant.tsx
Define chart dimension schemas in Constant file                   
+172/-0 
SelectedBlockPanel.tsx
Replace BlockAvatar with VariationIcon                                     
+7/-1     
BlocksWorkspaceActions.tsx
Remove visual flag from export JSON                                           
+8/-0     
Configuration changes
1 files
VisualMapConstant.tsx
Introduce VisualMapConstant with chart definitions             
+1107/-0
Bug fix
2 files
Pie.tsx
Fix Pie selector presence condition                                           
+5/-2     
Gantt.tsx
Use correct data.columns length check                                       
+1/-1     
Documentation
1 files
default-menu.ts
Update comment on VisualMapConstant export                             
+1/-1     
Additional files
5 files
FrameOperations.tsx +0/-412 
Line.tsx +1/-1     
MapChartBlockSettings.tsx +0/-1     
MapSelector.ts +0/-3     
ScatterPlotSelector.ts +0/-3     

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

    Potential Memory Leak

    The dispatchData function uses setTimeout to debounce updates but does not clear outstanding timers on component unmount, leading to possible memory leaks or state updates on unmounted components.

    }
    function dispatchData(option) {
        if (timeoutRef.current) {
            clearTimeout(timeoutRef.current);
            timeoutRef.current = null;
        }
        timeoutRef.current = setTimeout(() => {
            try {
                // set the value
                setData("option", option as PathValue<any, typeof path>);
            } catch (e) {
                console.log(e);
            }
        }, 100);
    }
    Repeated JSON.parse Usage

    The formattedColumns function and various variation handlers call JSON.parse(computedValue) repeatedly without error handling. Invalid or empty JSON could throw and crash the component.

    let tempVal = JSON.parse(computedValue) || {};
    const seriesIndex =
    Typo in Prop Name

    The prop formmattedColumns is misspelled (double "m"). Renaming it to formattedColumns would improve consistency and readability across the codebase.

    <D extends BlockDef = BlockDef>({ id, updateFrame, path, dragdropColumns, deleteColumns, formmattedColumns, isAdd, syncHeader, chart, storedColumns, visual, selectedItem }) => {

    @QodoAI-Agent
    Copy link
    Copy Markdown

    QodoAI-Agent commented Apr 23, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 45b106b

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Remove prop array mutation

    Avoid directly mutating the storedColumns prop array, which can cause unpredictable
    side effects. Instead clear related state via setters (setSelectedColumns and a
    callback) and allow the parent to update its own state.

    libs/renderer/src/components/block-defaults/echart-visualization-block/variant/bar-chart/DataTabStyling.tsx [125-130]

     const handleSelectedItem = (item: any) => {
         selectedItem(item);
         setSelectedColumns({});
    -    storedColumns.length = 0; // Clear the storedColumns array
    +    // Clear drag-and-drop mapping without mutating props
         Object.keys(dragdropColumns).forEach((key) => delete dragdropColumns[key]);
     };
    Suggestion importance[1-10]: 7

    __

    Why: Directly mutating the storedColumns prop array can cause unpredictable side effects; removing that mutation and relying on state setters preserves data integrity.

    Medium
    General
    Extract styled declarations

    Move styled component declarations outside the functional component to avoid
    re-creating them on every render and prevent stale closures. Define these once at
    module scope. This improves performance and ensures style definitions are stable.

    libs/renderer/src/components/block-defaults/echart-visualization-block/VisualMap.tsx [11-15]

    +// move above the component
     const StyledMain = styled("div")(() => ({
         width: "100%",
         height: "100%",
         marginTop: "1px",
     }));
    Suggestion importance[1-10]: 6

    __

    Why: Defining StyledMain inside the component causes it to be recreated on every render; moving it to module scope improves performance and stability.

    Low
    Focus input on mount

    The focus effect only needs to run once on mount, not every time the search string
    changes. Replace the timeout logic with a direct focus call inside an effect that
    has an empty dependency array.

    libs/renderer/src/components/block-defaults/echart-visualization-block/VisualMap.tsx [39-43]

     useEffect(() => {
    -    setTimeout(() => {
    -        searchInputRef.current?.focus();
    -    }, 0);
    -}, [search]);
    +    searchInputRef.current?.focus();
    +}, []);
    Suggestion importance[1-10]: 5

    __

    Why: Running the focus effect on every search change is unnecessary; using an empty dependency array focuses once on mount without altering functionality.

    Low

    Previous suggestions

    Suggestions up to commit 263e85e
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Update block settings via setter

    Use the provided setData updater from useBlockSettings instead of mutating the data
    object directly to ensure state is persisted correctly.

    libs/renderer/src/components/block-defaults/echart-visualization-block/VisualizationBlockMenu.tsx [90-96]

     const handleSelectedItem = (item: any) => {
         if (item.title && item.option) {
    -        data.variation = item.title;
    -        data.option = item.option;
    -        setForceRender(prev => !prev); // Force re-render to update the chart with the new data
    +        setData(prev => ({
    +            ...prev,
    +            variation: item.title,
    +            option: item.option
    +        }));
         }
     };
    Suggestion importance[1-10]: 9

    __

    Why: Using setData ensures the state update is persisted and reactive, avoiding direct mutation of data that won’t trigger proper updates.

    High
    Early exit on insufficient columns

    Add an early exit guard for formattedColumns to prevent accessing undefined columns
    when fewer than two columns are provided.

    libs/renderer/src/components/block-defaults/echart-visualization-block/variant/FrameOperations.tsx [176-178]

     const formattedColumns = (columnsValue: any[], variation: any) => {
    +    if (!columnsValue || columnsValue.length < 2) {
    +        return;
    +    }
         // check if the columns value has any values
         const hasValues = columnsValue.some(item => item?.values && item?.values.length > 0);
    Suggestion importance[1-10]: 6

    __

    Why: Adding a guard for when fewer than two columns are provided prevents undefined access in later logic, improving stability.

    Low
    Guard block iteration safety

    Guard against json.blocks being undefined before iterating and simplify optional
    chaining to prevent runtime errors.

    packages/client/src/components/blocks-workspace/BlocksWorkspaceActions.tsx [61-67]

    -Object.keys(json?.blocks).forEach((key) => {
    -    if (key.startsWith('e-chart')) {
    -        if (json?.blocks[key]?.data?.option?.['visual']) {
    -            json.blocks[key].data.option['visual'] = false;
    -        }
    +if (json?.blocks) {
    +  Object.keys(json.blocks).forEach((key) => {
    +    const block = json.blocks[key];
    +    if (key.startsWith('e-chart') && block.data?.option?.visual) {
    +      block.data.option.visual = false;
         }
    -});
    +  });
    +}
    Suggestion importance[1-10]: 5

    __

    Why: Adding a guard for json.blocks prevents a runtime error when blocks is undefined and streamlines the optional chaining, improving stability with minimal changes.

    Low
    General
    Use explicit image flag

    The item.icon ReactNode won’t have a reliable type property at runtime; add an
    explicit isImage flag to each item or detect via React.isValidElement and
    element.props.src.

    libs/renderer/src/components/block-defaults/echart-visualization-block/VisualMap.tsx [149-153]

     <span
         style={{
    -        marginLeft: item.icon?.type === "img" ? "30px" : "38px",
    +        marginLeft: item.isImage ? "30px" : "38px",
             display: "flex",
             alignItems: "center",
         }}
     >
    Suggestion importance[1-10]: 7

    __

    Why: Relying on a ReactNode’s type is unreliable at runtime; adding an explicit flag or better detection makes layout logic more robust.

    Medium
    Fix selectedColumn state type

    Align the selectedColumn state type with the actual data shape or define a proper
    interface instead of using string[].

    libs/renderer/src/components/block-defaults/echart-visualization-block/VisualizationBlockMenu.tsx [80-89]

    -const [selectedColumn, setSelectedColumn] = useState<string[]>([]);
    +const [selectedColumn, setSelectedColumn] = useState<any[]>([]);
     function handleStoreData(storeData: any[]) {
         const hasValues = storeData.some(item => item?.values && item?.values.length > 0);
         if (hasValues) {
             setSelectedColumn(storeData);
         }
     }
    Suggestion importance[1-10]: 5

    __

    Why: Changing the state’s type to match the actual data prevents type mismatches and potential runtime issues, improving type safety.

    Low
    Wrap JSON.parse in try/catch

    Wrap JSON.parse in a try/catch to handle invalid JSON and avoid runtime exceptions.

    libs/renderer/src/components/block-defaults/echart-visualization-block/variant/FrameOperations.tsx [246]

    -let tempVal = JSON.parse(computedValue) || {};
    +let tempVal;
    +try {
    +    tempVal = JSON.parse(computedValue) || {};
    +} catch {
    +    tempVal = {};
    +}
    Suggestion importance[1-10]: 5

    __

    Why: Wrapping JSON.parse in a try/catch avoids runtime crashes on malformed JSON, enhancing error resilience.

    Low
    Add image alt attribute

    Add an alt attribute to the image for accessibility compliance.

    packages/client/src/components/blocks-workspace/panels/SelectedBlockPanel.tsx [264]

    -<StyledVariationIcon src={VariationIcon} />
    +<StyledVariationIcon src={VariationIcon} alt="Variation logo" />
    Suggestion importance[1-10]: 4

    __

    Why: Including an alt attribute on the img element enhances accessibility by providing descriptive text for screen readers.

    Low

    @ehynd
    Copy link
    Copy Markdown
    Contributor

    ehynd commented May 12, 2025

    @Gowrishankar-Palanisamy can you pause on your current ticket, merge dev into this, and take a look at the errors? a few changes made in the past few weeks are affecting this!

    @ehynd ehynd mentioned this pull request May 14, 2025
    @ehynd
    Copy link
    Copy Markdown
    Contributor

    ehynd commented May 14, 2025

    @bannaarisamy-shanmugham-kanini could you please merge your branch from #950 into this branch? and @Gowrishankar-Palanisamy could you both then please work together to pull in dev and make sure everything is functioning properly? would like to have these two on one branch since there have been a few changes and want to ensure the code changes work with each other. thank you!

    @Gowrishankar-Palanisamy
    Copy link
    Copy Markdown
    Contributor Author

    Merged this PR with tools tab PR #967 and also solved the merge conflict

    @johbaxter johbaxter merged commit cb5f520 into dev May 19, 2025
    3 checks passed
    @johbaxter johbaxter deleted the feat/enhance-data-tab branch May 19, 2025 15:37
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-05-19 *

    Changed

    • Data tab styling across all chart variants enhanced
    • Integrated VisualMap popover for streamlined visual selection
    • Updated rendering and styling of bar, line, scatter, pie and stack charts

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

    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    • When i bringing my chart back into edit mode we need to see the columns i had selected previously
    • disable the chart select options we don't offer

    @ehynd you know this

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-05-19 *

    Changed

    • Updated data tab styling across existing chart variants (bar, line, scatter, pie, stack) for improved consistency and future extensibility.

    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.

    Update Data Tab in Charts

    6 participants