Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: explicit mouse move event user action #6549

Merged
merged 4 commits into from
Nov 27, 2024

Conversation

vikrantgupta25
Copy link
Collaborator

@vikrantgupta25 vikrantgupta25 commented Nov 27, 2024

Summary

  • when the user enters the dropdown then post the first time we block the mouse enter event to handle the re-render keyboard scrolling better.
  • but this caused issue for later explicit mouse enter events where user actually does some action
  • so we capture these explicit events from mouse move.

Important

Capture explicit mouse move events and centralize column selection logic in LogsFormatOptionsMenu.tsx.

  • Behavior:
    • Capture explicit mouse move events in LogsFormatOptionsMenu.tsx to handle user actions better.
    • Use onMouseMove to update selectedValue for explicit mouse actions.
  • Functions:
    • Introduce handleColumnSelection() to centralize column selection logic.
    • Replace inline selection logic in handleKeyDown() and onClick with handleColumnSelection().

This description was created by Ellipsis for 02af141. It will automatically update as commits are pushed.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the bug Something isn't working label Nov 27, 2024
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to efa5795 in 36 seconds

More details
  • Looked at 15 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:294
  • Draft comment:
    Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This is applicable in multiple places in this file.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_H5s9MErEcXLGAIRL


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

2 similar comments
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on e89b64b in 54 seconds

More details
  • Looked at 110 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:172
  • Draft comment:
    The condition if (currentIndex - 1 > 0) should be if (currentIndex - 1 >= 0) to correctly handle the case when there is only one element.
  • Reason this comment was not posted:
    Marked as duplicate.
2. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:314
  • Draft comment:
    The condition if (currentIndex - 1 > 0) should be if (currentIndex - 1 >= 0) to correctly handle the case when there is only one element.
  • Reason this comment was not posted:
    Marked as duplicate.
3. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:312
  • Draft comment:
    Avoid using inline styles. Replace with CSS classes or styled components. This applies to other inline styles in this file as well.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_7leqxrch1OAwl4Oe


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on ee1d153 in 27 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:125
  • Draft comment:
    The condition currentIndex - 1 > 0 should be currentIndex - 1 >= 0 to correctly handle the case where the first element is selected. This change is already made in the PR.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_Y70EWzcs193RoRA7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 02af141 in 33 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:85
  • Draft comment:
    Ensure addColumn is always defined when handleToggleAddNewColumn is called to avoid potential runtime errors.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The addition of addColumn?.onSearch?.(''); in handleToggleAddNewColumn is a good change to ensure the search is reset when toggling the column container. However, it's important to ensure that addColumn is always defined when this function is called to avoid potential runtime errors.
2. frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx:84
  • Draft comment:
    Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is applicable to the LogsFormatOptionsMenu component.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_uAMvC9smiOupvbqx


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@vikrantgupta25 vikrantgupta25 merged commit 813cd84 into develop Nov 27, 2024
13 of 15 checks passed
@vikrantgupta25 vikrantgupta25 deleted the handle-explicit-mouse-move branch November 27, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants