Skip to content

[Lens] Data panel styling#39589

Closed
flash1293 wants to merge 73 commits intoelastic:feature/lensfrom
flash1293:lens/data-panel-styling
Closed

[Lens] Data panel styling#39589
flash1293 wants to merge 73 commits intoelastic:feature/lensfrom
flash1293:lens/data-panel-styling

Conversation

@flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jun 25, 2019

Depends on #39587

This PR styles the data panel like in the POC and adds type filters and search bar.

Screenshot 2019-06-27 at 15 09 12

The style of search/filter is consistent with dashboard add panel

Screenshot 2019-06-27 at 15 24 08

If there are no index patterns, a warning is shown.

Screenshot 2019-06-27 at 15 25 01

Long field names wrap on dots

Screenshot 2019-06-27 at 15 17 16

wylieconlon and others added 30 commits May 30, 2019 18:41
@flash1293 flash1293 changed the title [skip ci] [WIP] [Lens] Data panel styling [WIP] [Lens] Data panel styling Jun 27, 2019
@flash1293
Copy link
Contributor Author

Jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@flash1293 flash1293 changed the title [WIP] [Lens] Data panel styling [Lens] Data panel styling Jun 28, 2019
@flash1293 flash1293 marked this pull request as ready for review June 28, 2019 09:49
@flash1293 flash1293 requested a review from a team as a code owner June 28, 2019 09:49
@flash1293
Copy link
Contributor Author

flash1293 commented Jun 28, 2019

@ryankeairns Another one for you if you have time :)

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't done a thorough review yet but I'm finding that this basically crashes my browser when I switch to the metricbeat index pattern which has thousands of fields. Also, the field list is not scrollable when the list is too long.

button={
<EuiButtonIcon
aria-label="Switch to datasource"
title="Switch to datasource"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please i18n

>;

function sortFields(fieldA: IndexPatternField, fieldB: IndexPatternField) {
return fieldA.name.toLowerCase() < fieldB.name.toLowerCase() ? -1 : 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been preferring String.localeCompare for this because lowercase is not the same in all languages: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/localeCompare

@flash1293
Copy link
Contributor Author

I haven't done a thorough review yet but I'm finding that this basically crashes my browser when I switch to the metricbeat index pattern which has thousands of fields. Also, the field list is not scrollable when the list is too long.

Good point @wylieconlon I didn't test with long field lists. We could probably use virtualized scrolling here, I will try to add this tomorrow.

@flash1293
Copy link
Contributor Author

@wylieconlon I looked into this a bit further and it seems like there is not really a viable way around using a lib like react-virtualize. Even with the completely unstyled list switching to the metricbeat index pattern takes 1-2s on a macbook. The old visualize editor uses combo box everywhere which does the same thing under the hood.

I will continue working on this, if you or @chrisdavies want to take this PR over that's also fine for me.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@flash1293
Copy link
Contributor Author

Closing this as it's bundled into #40787
Thanks for picking this up, @chrisdavies !

@flash1293 flash1293 closed this Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Lens release_note:skip Skip the PR/issue when compiling release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants