DataTable: Fix and Simplify overlay handling in cell edit mode #7951
Conversation
|
It appears that all code related to overlayService and the selfClickRef is only used in this specific context. I would update this PR to remove that logic—unless this general approach is not acceptable. Otherwise, I’ll push the remaining changes shortly. |
|
@gnawjaren may as well push all your changes. |
|
Conflicts now. Is this PR still needed? |
|
That depends on whether this alternative approach is considered desirable or an improvement over the current fix. |
|
OK i will leave it pending review. But you should fix the merge conflicts. |
|
@gnawjaren can you fix the merge conflicts? I have also linked this PR to that new ticket. |
5a6a89c to
453f589
Compare
|
I’ve rebased the branch and resolved the conflicts. I also re-tested the changes in our project. |
|
@gnawjaren yes please i think so this does simplify and clean up the handling. |
|
@gnawjaren could you please fix the merge conflicts once again? Let's merge this long awaiting change to the next release :) |
2ca6aa2 to
e227522
Compare
e227522 to
a68155f
Compare
|
@melloware Is there anything more we can do here or can we merge it? |
|
I don’t think this is the right solution. I believe DomHandler.getParents will affect the table’s performance. Also, OverlayService is an exported eventbus, so users can integrate a select or similar component from a different library using this OverlayService.on('overlay-click') |
|
I understand your concerns about performance. getParents only runs on document clicks and only when an overlay is open, The improved third-party overlay support is more of a side benefit — Ultimately, I believe this trade-off is worth it, |
Defect Fixes
Simplify overlay handling in cell edit mode
This PR simplifies the current overlay handling logic in cell edit mode by introducing a declarative approach.
Instead of relying on OverlayService, selfClick, and setTimeout workarounds, it checks for a data-pr-is-overlay attribute on clicked elements (or their parents) to determine whether an outside click should close the cell editor.
Benefits:
• Greatly reduces complexity and improves maintainability
• More robust and easier to debug
• Enables support for custom or third-party overlay editors, including full interaction, as long as they include the data-pr-is-overlay attribute on their container
• Makes the logic more declarative and extensible
I tested this in various parts of our project with good results.
Let me know if this approach is acceptable – I’d be happy to adjust the implementation if needed.
Fix #7405
Fix #7158
Fix #8080