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 filterPanel position bug when table columns is changed #12654

Closed
wants to merge 1 commit into from

Conversation

hectorqin
Copy link

@hectorqin hectorqin commented Sep 6, 2018

The position of filterPanel is wrong when the table header column is changed.So pass the event target to popperJS.

#6159

  • Make sure you follow Element's contributing guide (中文 | English | Español).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.

@element-bot
Copy link
Member

Deploy preview for element ready!

Built with commit c958632

https://deploy-preview-12654--element.netlify.com

@ziyoung
Copy link
Contributor

ziyoung commented Sep 26, 2018

@hectorqin 使用最新的 ElementUI,并不能重现这个问题。https://jsfiddle.net/41cc3ewk/8/

@hectorqin
Copy link
Author

@ziyoung 试试这个。https://jsfiddle.net/41cc3ewk/10/
复现步骤: 点击筛选 > 点击toggle > 点击筛选,筛选框位置错误。

@hectorqin hectorqin changed the title Fix filterPanel position bug when table is changed Fix filterPanel position bug when table columns is changed Sep 26, 2018
@ziyoung
Copy link
Contributor

ziyoung commented Sep 27, 2018

@hectorqin 这个 pr 是可以解决问题,但是我觉得存在一些问题。
使用 vue-popper 时,一个 reference 元素对应一个 popper 元素。在这本例中,因为 reference 元素改变了(实际上重新渲染了)导致 popper 的位置计算不正确。你目前的做法是直接修改 reference。但正确的做法应该是每当 reference 改变,都要重新执行以下 vue-popper 里面的相关逻辑才对。直接修改 reference 可行,但也许会造成一些未知的错误。

@hectorqin
Copy link
Author

hectorqin commented Sep 28, 2018

@ziyoung 你可以在src/utils/popper.js里面搜索_reference,里面涉及到的逻辑就是获取_reference元素的位置信息和父滚动元素,不涉及到需要初始化的逻辑。而在 vue-popper 中 referenceElm 元素 也只是用来初始化popper,并没有其它逻辑。

@ziyoung ziyoung closed this Sep 28, 2018
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.

3 participants