[Lens] Consistent Drag and Drop styles by cchaos · Pull Request #78674 · elastic/kibana

cchaos · 2020-09-28T19:50:24Z

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:

Min-height for the workspace area
Using EuiHighlight for the search term highlighting in the fields panel instead of the custom UI

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility

This reverts commit 2fddd51.

But moving style isn’t working

cchaos

@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/drag_drop/drag_drop.tsx

x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx

flash1293 · 2020-09-29T15:23:28Z

@elasticmachine merge upstream

flash1293

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 :-)

x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx

x-pack/plugins/lens/public/drag_drop/drag_drop.tsx

flash1293 · 2020-09-30T08:49:29Z

@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?

cchaos · 2020-09-30T13:39:41Z

Thanks @flash1293. It's all working and looking great now. I'll pull out of drafts.

wylieconlon

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.

…dnd-design

wylieconlon

Reviewed and tested in Chrome and Firefox. The new changes LGTM, including performance of the Eui highlight on large index patterns. I found that there is an existing styling issue in the drag styling for the Filters and Ranges editors:

wylieconlon · 2020-09-30T18:13:24Z

x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss


 .lnsLayerPanel__dimension {
  @include euiFontSizeS;
-  background: lightOrDarkTheme($euiColorEmptyShade, $euiColorLightestShade);


I was a bit surprised to see how low-contrast the empty dimensions look now, with gray on gray text. Would you consider choosing a darker text color or lighter background color, or do you think the low contrast is effective here?

Yes the low contrast here was intentional as an additional support of #76021 and as indicated in the mocks in #75938. It further helps distinguish between filled and unfilled dimensions. The contrast itself is still sufficient to meet a11y.

x-pack/plugins/lens/public/drag_drop/drag_drop.tsx

cchaos · 2020-09-30T18:27:06Z

I found that there is an existing styling issue in the drag styling for the Filters and Ranges editors

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.

cchaos · 2020-10-01T13:26:46Z

@elasticmachine merge upstream

kibanamachine · 2020-10-01T15:39:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a2267b4

Metrics [docs]

async chunks size

id	before	after	diff
`lens`	1002.8KB	1007.4KB	+4.6KB

page load bundle size

id	before	after	diff
`kibanaReact`	123.3KB	124.4KB	+1.0KB

History

💔 Build #78739 failed 21397d9
💚 Build #78575 succeeded f57e106
💚 Build #78408 succeeded ff803d3
💚 Build #78286 succeeded a6c954e
💚 Build #78196 succeeded 645974d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

elasticmachine · 2020-10-01T17:40:55Z

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) ...

cchaos and others added 12 commits September 22, 2020 16:39

Working commit

365de55

Drop illustration working for empty and non-empty including hover

6641e62

Remove wrapping div of DragDrop and pass props to child

728a361

Using FieldButton for empty dimension

2fddd51

Using EuiHighlight

58369e3

Revert "Using FieldButton for empty dimension"

cf5fab5

This reverts commit 2fddd51.

Basic styles in for all DnD states

0ea8634

Fixing dimension button styles

371f73c

Finished up styles

8de1bb4

But moving style isn’t working

Merge branch 'lens/design-for-dnd' into lens/dnd-design

7be46c5

Cleanup

ab4502d

Trying to fix some tests

bdf937d

cchaos requested a review from flash1293 September 28, 2020 19:51

i18n

b791e64

cchaos commented Sep 28, 2020

View reviewed changes

x-pack/plugins/lens/public/drag_drop/drag_drop.tsx Outdated Show resolved Hide resolved

x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx Outdated Show resolved Hide resolved

Fix a type

88403e2

Merge branch 'master' into lens/dnd-design

2926d4e

flash1293 reviewed Sep 29, 2020

View reviewed changes

x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx Show resolved Hide resolved

x-pack/plugins/lens/public/drag_drop/drag_drop.tsx Show resolved Hide resolved

x-pack/plugins/lens/public/drag_drop/drag_drop.tsx Outdated Show resolved Hide resolved

cchaos and others added 5 commits September 29, 2020 16:07

Fix FieldButton to accept …rest props

7763ad3

A few other minor fixes

a788053

stuff

97ddc8c

Merge remote-tracking branch 'upstream/master' into lens/dnd-design

0588fcd

fix bugs and styles

645974d

One last fix to fields list

a6c954e

cchaos marked this pull request as ready for review September 30, 2020 13:39

cchaos requested a review from a team September 30, 2020 13:39

cchaos requested a review from a team as a code owner September 30, 2020 13:39

cchaos added Feature:Lens v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 30, 2020

wylieconlon added 2 commits September 30, 2020 11:40

Merge remote-tracking branch 'origin/master' into HEAD

cd005c3

Add scss imports to guarantee mixins are available

f59b0ed

wylieconlon reviewed Sep 30, 2020

View reviewed changes

Merge branch 'lens/dnd-design' of github.com:cchaos/kibana into lens-…

ff803d3

…dnd-design

wylieconlon approved these changes Sep 30, 2020

View reviewed changes

Fixed horizontal scroll of error message

f57e106

cchaos mentioned this pull request Oct 1, 2020

[Lens] fix add new dimension button click area #79098

Closed

elasticmachine and others added 2 commits October 1, 2020 09:26

Merge branch 'master' into lens/dnd-design

21397d9

Quick fix for invalid link

a2267b4

cchaos merged commit e248f32 into elastic:master Oct 1, 2020

cchaos mentioned this pull request Oct 1, 2020

[7.x] [Lens] Consistent Drag and Drop styles (#78674) #79153

Merged

wylieconlon added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Oct 1, 2020

cchaos deleted the lens/dnd-design branch October 1, 2020 18:48

Conversation

cchaos commented Sep 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Closes #75938 & closes #78643

Other quick fixes:

Checklist

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

flash1293 commented Sep 29, 2020

Uh oh!

flash1293 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flash1293 commented Sep 30, 2020

Uh oh!

cchaos commented Sep 30, 2020

Uh oh!

wylieconlon left a comment

Choose a reason for hiding this comment

Uh oh!

wylieconlon left a comment

Choose a reason for hiding this comment

Uh oh!

wylieconlon Sep 30, 2020

Choose a reason for hiding this comment

Uh oh!

cchaos Sep 30, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cchaos commented Sep 30, 2020

Uh oh!

cchaos commented Oct 1, 2020

Uh oh!

kibanamachine commented Oct 1, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

page load bundle size

History

Uh oh!

elasticmachine commented Oct 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cchaos commented Sep 28, 2020 •

edited

Loading

flash1293 left a comment •

edited

Loading