Skip to content

1192 line chart filter#1232

Merged
johbaxter merged 2 commits intodevfrom
1192-LineChartFilter
May 30, 2025
Merged

1192 line chart filter#1232
johbaxter merged 2 commits intodevfrom
1192-LineChartFilter

Conversation

@Manikandan-Kanini
Copy link
Copy Markdown
Contributor

Description

Changes Made

How to Test

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

Notes

@Manikandan-Kanini Manikandan-Kanini requested a review from a team as a code owner May 29, 2025 08:40
@Manikandan-Kanini Manikandan-Kanini linked an issue May 29, 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

1192 line chart filter


User description

Description

Changes Made

How to Test

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

Notes


PR Type

Enhancement


Description

  • Refine line chart brush filtering logic.

  • Compute indices from coordinate ranges.

  • Update brush event to 'brushEnd'.

  • Restrict toolbox brush menu types.


Changes walkthrough 📝

Relevant files
Enhancement
Line.tsx
Refine line chart brush selection logic                                   

libs/renderer/src/components/block-defaults/echart-visualization-block/variant/line-chart/Line.tsx

  • Switched chart event from 'brushSelected' to 'brushEnd'
  • Extracted x and y ranges from brush areas
  • Computed indices intersection for filtering
  • Applied combined frame.filter with series and axis
  • +31/-20 
    Configuration changes
    default-menu.ts
    Restrict brush menu types to rectangle                                     

    packages/client/src/components/blocks-workspace/menus/default-menu.ts

  • Removed polygon, lineX, lineY brush types
  • Restricted brush types to ['rect','clear']
  • +1/-7     

    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 State Fields

    Accessing resultData["_state"]["fields"]["yAxis"] without checking existence may cause runtime errors if fields or yAxis is undefined.

    const yAxisListLength =
        resultData["_state"]["fields"]["yAxis"].length;
    Event Handler Leak

    Attaching the 'brushEnd' listener inside a debounced loader without cleanup can register multiple handlers over time.

    const echartsLoaded = debounce((chart) => {
        // Fires only once when brush is released
        chart.on("brushEnd", (params) => {
            if (!params.areas || !params.areas.length) return;
            const area = params.areas[0];
            // Get xAxis data
            const currentOption = chart.getOption();
            const labelData = currentOption.series[0].data || [];
            const xAxisData = currentOption.xAxis?.[0]?.data || [];
            let indices = [];
            if (area.coordRange && area.coordRange.length === 2) {
                const [xRange, yRange] = area.coordRange;
                const xIndices = [];
                for (let i = xRange[0]; i <= xRange[1]; i++) xIndices.push(i);
                const yIndices = [];
                for (let i = 0; i < labelData.length; i++) {
                    const val = labelData[i];
                    if (val >= yRange[0] && val <= yRange[1]) yIndices.push(i);
                }
                indices = xIndices.filter((i) => yIndices.includes(i));
    
                const filteredLabels = indices.map((i) => xAxisData[i]);
                const filteredValues = indices.map((i) => labelData[i]);
    
                if (filteredValues.length > 0) {
                    frame.filter(
                        `SetFrameFilter(((${currentOption.series[0]?.name}==[${filteredValues}]) AND (${currentOption.xAxis?.[0]?.name}==[${filteredLabels}])))`,
                    );
                }
            }
        });
    }, 2000);
    Filter Injection Risk

    Constructing filter strings by interpolating unescaped series names and values may lead to invalid syntax or unintended behavior.

        frame.filter(
            `SetFrameFilter(((${currentOption.series[0]?.name}==[${filteredValues}]) AND (${currentOption.xAxis?.[0]?.name}==[${filteredLabels}])))`,
        );
    }

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Clamp and integer-round brush indices

    Ensure the brush range indices are valid integers and within the bounds of the data
    array to avoid out-of-range or fractional index errors. Clamp and round the
    start/end values before iterating.

    libs/renderer/src/components/block-defaults/echart-visualization-block/variant/line-chart/Line.tsx [200]

    -for (let i = xRange[0]; i <= xRange[1]; i++) xIndices.push(i);
    +const startIdx = Math.max(0, Math.ceil(xRange[0]));
    +const endIdx = Math.min(labelData.length - 1, Math.floor(xRange[1]));
    +const xIndices: number[] = [];
    +for (let i = startIdx; i <= endIdx; i++) {
    +    xIndices.push(i);
    +}
    Suggestion importance[1-10]: 7

    __

    Why: This guards against out-of-range or fractional brush indices by clamping and rounding, improving robustness without altering core functionality.

    Medium
    Safely extract numeric point values

    Safely extract a numeric value from each data point in case the series items are
    objects. Fallback to the raw value if no numeric value property exists.

    libs/renderer/src/components/block-defaults/echart-visualization-block/variant/line-chart/Line.tsx [202-205]

     for (let i = 0; i < labelData.length; i++) {
    -    const val = labelData[i];
    -    if (val >= yRange[0] && val <= yRange[1]) yIndices.push(i);
    +    const raw = labelData[i];
    +    const val = typeof raw === 'object' && raw.value != null ? raw.value : raw;
    +    if (val >= yRange[0] && val <= yRange[1]) {
    +        yIndices.push(i);
    +    }
     }
    Suggestion importance[1-10]: 5

    __

    Why: Extracting numeric values from possible object data adds safety for varied series item formats, but the impact is limited if data is already primitive.

    Low
    General
    Serialize filter arrays correctly

    Use JSON.stringify for the arrays in the filter expression to ensure proper quoting
    and formatting of values.

    libs/renderer/src/components/block-defaults/echart-visualization-block/variant/line-chart/Line.tsx [212-214]

     frame.filter(
    -    `SetFrameFilter(((${currentOption.series[0]?.name}==[${filteredValues}]) AND (${currentOption.xAxis?.[0]?.name}==[${filteredLabels}])))`,
    +    `SetFrameFilter((${currentOption.series[0]?.name}==${JSON.stringify(filteredValues)} AND ${currentOption.xAxis?.[0]?.name}==${JSON.stringify(filteredLabels)}))`
     );
    Suggestion importance[1-10]: 7

    __

    Why: Using JSON.stringify ensures proper quoting and formatting in the frame filter expression, preventing potential parsing errors.

    Medium

    @johbaxter johbaxter merged commit 87796b2 into dev May 30, 2025
    4 checks passed
    @johbaxter johbaxter deleted the 1192-LineChartFilter branch May 30, 2025 16:20
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-05-30 *

    Changed

    • Add interactive brushing filter for line charts and restrict brush tool to rectangle and clear

    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.

    Line chart brush event

    3 participants