-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Consistent Drag and Drop styles #78674
Conversation
This reverts commit 2fddd51.
But moving style isn’t working
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.
@flash1293 Not much needed for help here except for one test and one prop that is not updating with DnD
x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx
Outdated
Show resolved
Hide resolved
@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.
Noticed a few things, I will fix the update problem of the dragdrop component tomorrow. For the other things commented on below I think you know than me already :-)
@cchaos I merged master and implemented the fix with a custom equality check function. I had to change the logic for applying classnames a little, otherwise css properties would get overwritten. It should work fine now: Can you take another look whether everything still works as you intended? |
Thanks @flash1293. It's all working and looking great now. I'll pull out of drafts. |
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.
Just pushed a fix to fix an SCSS failure I found:
SassError: SassError: no mixin named lnsDroppable
on line 70 of /Users/wylie/dev/kibana/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss
>> @include lnsDroppable;
I'll be continuing to review the code and behavior.
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.
@@ -27,36 +27,41 @@ | |||
|
|||
.lnsLayerPanel__dimension { | |||
@include euiFontSizeS; | |||
background: lightOrDarkTheme($euiColorEmptyShade, $euiColorLightestShade); |
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.
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.
I actually see that more as a feature than an issue because this particular DnD interaction is restricted to it's own panel while the others aren't. So it helps reinforce this containment. |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]async chunks size
page load bundle size
History
To update your PR or re-run it, just comment with: |
Pinging @elastic/kibana-app (Team:KibanaApp) |
* Remove wrapping div of DragDrop and pass props to child * Using EuiHighlight * Basic styles in for all DnD states * Fixing dimension button styles * Fix FieldButton to accept `…rest` props * A few other minor fixes * Fixed horizontal scroll of error message * Quick fix for invalid link
…aly-detection-partition-field * 'master' of github.com:elastic/kibana: (76 commits) Fix z-index of KQL Suggestions dropdown (elastic#79184) [babel] remove unused/unneeded babel plugins (elastic#79173) [Search] Fix timeout upgrade link (elastic#79045) Always Show Embeddable Panel Header in Edit Mode (elastic#79152) [Ingest]: add more test for transform index (elastic#79154) [ML] DF Analytics: Collapsable sections on results pages (elastic#76641) [Fleet] Fix agent policy change action migration (elastic#79046) [Ingest Manager] Match package spec `dataset`->`data_stream` and `config_templates`->`policy_templates` renaming (elastic#78699) Revert "[Metrics UI] Add ability to override datafeeds and job config for partition field (elastic#78875)" [ML] Update transform cloning to include description and new fields (elastic#78364) chore(NA): remove non existing plugin paths from case api integration tests (elastic#79127) [Ingest Manager] Ensure we trigger agent policy updated event when we bump revision. (elastic#78836) [Metrics UI] Display No Data context.values as [NO DATA] (elastic#78038) [Monitoring] Missing data alert (elastic#78208) [Lens] Fix embeddable title and description for reporting and dashboard tooltip (elastic#78767) [Lens] Consistent Drag and Drop styles (elastic#78674) [ML] Model management UI fixes and enhancements (elastic#79072) [Metrics UI] Add ability to override datafeeds and job config for partition field (elastic#78875) [Security Solution]Fix basepath used by endpoint telemetry tests (elastic#79027) update rum agent version which contains longtasks (elastic#79105) ...
Closes #75938 & closes #78643
The biggest change is that instead of the DnD component wrapping it's children in another layer, it now clones it and just applies the extra classes and handlers. This enabled me to provide the classes at the appropriate levels (closes to the button).
And they styles now all have the same coloring, transition timings, etc. It also moved a lot of the logic of copy vs move to the DnD component itself so that we can be consistent moving forward.
GIF: https://share.getcloudapp.com/jkuDrzJQ
Other quick fixes:
Checklist
Delete any items that are not applicable to this PR.