-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Thresholds: moving a threshold to another group should carry also its styling #112853
Conversation
Pinging @elastic/kibana-vis-editors (Team:VisEditors) |
@elasticmachine merge upstream |
Updated PR to copy styles also in the duplicate scenario. |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and it works as expected, now let's do the same for data layers 💪🏼 😄
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
…lso its styling (elastic#112853) * 🐛 When dragging a threshold to another group carry also its styling * ✅ Add functional test * ✨ Make duplicate carry style too * ✅ Add functional test for duplicate use case * 🐛 Fix table duplication issue Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…lso its styling (#112853) (#113451) * 🐛 When dragging a threshold to another group carry also its styling * ✅ Add functional test * ✨ Make duplicate carry style too * ✅ Add functional test for duplicate use case * 🐛 Fix table duplication issue Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Marco Liberati <[email protected]>
Summary
Fixes #112799
This PR addresses the specific behaviour of moving a dimension from one group to another, where only datasource data was copied, ignoring visualization styling.
The new code is now adding one more case to the
previousColumn
assignment, passing thecolumnId
also in the case of move.I thought about limiting this specific behaviour only to threshold data layers, but I see no particular harm for data layer as well: from my testing
datatable
(the only vis using thepreviousColumn
parameter)had no issue with this changehad some issues with this change, so now groups can express with a flag whether duplication action requires the previous column or not.Also: wanted to add some unit tests but probably with all the required mocking it is not worth. Added a functional test ✅ .
Checklist
Delete any items that are not applicable to this PR.