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(grid): [grid] Fix bug where dragging and dropping in the expanded state of the table cannot render correctly #2901

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

chenxi-20
Copy link
Collaborator

@chenxi-20 chenxi-20 commented Feb 14, 2025

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #1995

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Refactor
    • Streamlined grid row rendering for more consistent display and smoother expansion behavior.
    • Enhanced drag-and-drop functionality, ensuring more reliable and intuitive row reordering.

@chenxi-20 chenxi-20 added the bug Something isn't working label Feb 14, 2025
Copy link

coderabbitai bot commented Feb 14, 2025

Walkthrough

This change updates key assignment logic in the grid body component and refines drag-and-drop row handling. In the body component, key generation in renderRow and renderRowExpanded has been streamlined and adjusted for uniqueness. In the drag handler, the retrieval of the previous row element now accounts for expanded rows, and error handling during drag end events is enhanced. These updates aim to improve rendering consistency and the robustness of drag-and-drop operations within the grid.

Changes

File(s) Change Summary
packages/vue/src/grid/src/body/src/body.tsx - In renderRow, removed a conditional check and simplified key assignment to rowid.
- In renderRowExpanded, updated key to expand_${rowid}${rowIndex} for uniqueness.
packages/vue/src/grid/src/dragger/src/rowDrop.ts - Modified createHandlerOnEnd to destructure and assign previousElementSibling to prevTrElem more explicitly.
- Added a check for rows with the tiny-grid-body__expanded-row class to adjust element targeting and improve error handling.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant DragHandler as createHandlerOnEnd
    participant Grid as Grid Component

    User->>DragHandler: Ends drag event on row
    DragHandler->>Grid: Retrieve target <tr> element's previous sibling
    Grid-->>DragHandler: Return previousElementSibling
    DragHandler->>DragHandler: Check if element has 'tiny-grid-body__expanded-row' class
    DragHandler->>Grid: If expanded, retrieve its previous sibling
    DragHandler->>DragHandler: Validate drag operation (prevent self-child insertion)
    DragHandler-->>User: Finalize drag-and-drop operation
Loading

Possibly related PRs

Suggested reviewers

  • zzcr

Poem

I'm a bunny with a hop so light,
Updating keys with all my might.
Rows now dance in unique array,
Drag-and-drop now leads the way.
Carrots and code, a joyful sight!
🐇💻

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/vue/src/grid/src/body/src/body.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-vue".

(The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-vue@latest --save-dev

The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

packages/vue/src/grid/src/dragger/src/rowDrop.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-vue".

(The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-vue@latest --save-dev

The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 441c376 and 3511adc.

📒 Files selected for processing (2)
  • packages/vue/src/grid/src/body/src/body.tsx (3 hunks)
  • packages/vue/src/grid/src/dragger/src/rowDrop.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: PR E2E Test (pnpm test:e2e3)
  • GitHub Check: PR Unit Test
🔇 Additional comments (4)
packages/vue/src/grid/src/dragger/src/rowDrop.ts (2)

62-63: LGTM: Improved variable handling for drag operations.

The code now properly destructures and assigns the previous element, making the code more maintainable.


71-73: Fixed drag-and-drop with expanded rows.

The code now correctly handles cases where the previous element is an expanded row by skipping it and using its previous sibling instead. This fixes the rendering bug when dragging rows in an expanded table.

packages/vue/src/grid/src/body/src/body.tsx (2)

567-567: LGTM: Simplified row key generation.

The code now uses a simpler and more reliable key generation by directly using rowid. This is safe as rowid is already guaranteed to be unique.


611-611: Improved expanded row key uniqueness.

Adding rowIndex to the expanded row key ensures unique keys even when the same row is expanded in different sections of the table. This prevents potential rendering issues in Vue's virtual DOM diffing.

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Walkthrough

此PR修复了在表格展开状态下拖拽无法正确渲染的问题。主要修改包括调整表格行的key值以确保唯一性,并在拖拽操作中正确识别和处理展开行。

Changes

文件 概要
packages/vue/src/grid/src/body/src/body.tsx 修改了表格行的key值,确保在展开状态下的唯一性。
packages/vue/src/grid/src/dragger/src/rowDrop.ts 在拖拽逻辑中添加了对展开行的识别和处理。

@chenxi-20 chenxi-20 changed the title fix(grid): [grid] 修复表格展开状态下拖拽无法正确渲染bug fix(grid): [grid] Fix bug where dragging and dropping in the expanded state of the table cannot render correctly Feb 14, 2025
@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Walkthrough

This PR fixes the issue where dragging and dropping cannot render correctly when the table is expanded. The main modifications include adjusting the key value of the table row to ensure uniqueness and correctly identifying and processing the expanded rows in the drag and drop operation.

Changes

Documents Summary
packages/vue/src/grid/src/body/src/body.tsx Modify the key value of the table row to ensure uniqueness in the expanded state.
packages/vue/src/grid/src/dragger/src/rowDrop.ts Added recognition and processing of expanded rows in drag and drop logic.

@@ -897,7 +897,8 @@ export default defineComponent({
render() {
let { $parent: $table } = this as any
let { $grid, isCenterEmpty, keyboardConfig = {}, mouseConfig = {}, renderEmpty } = $table
let { scrollLoad, tableColumn, tableData, tableLayout } = $table
let { scrollLoad, tableColumn, tableLayout } = $table
const tableData = $table.$grid.data || $table.tableData
Copy link
Member

Choose a reason for hiding this comment

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

此处不应该修改,在编辑场景,$table.$grid.data是旧数据,$table.tableData是表格内置数据,两者是不一样的。拖拽的主要问题是因为key

@@ -608,7 +608,7 @@ function renderRowExpanded(args) {
'tr',
{
class: 'tiny-grid-body__expanded-row',
key: `expand_${rowid}`,
key: `expand_${rowid}${rowIndex}`,
Copy link
Member

Choose a reason for hiding this comment

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

这里不需要再加Index了吧,rowid已经可以正确区分key了。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants