Skip to content
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

Change indexed fields table to use EuiToolTip and display 10 rows by default #17074

Merged
merged 8 commits into from
Mar 13, 2018

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Mar 9, 2018

Updates EUI to 0.0.26 to use some cool new stuff.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chrisronline
Copy link
Contributor

Can you post some screenshots of what's different?

@jen-huang
Copy link
Contributor Author

@chrisronline:

  • Table now displays 10 rows by default
  • Tooltip is now working correctly (replaced TooltipTrigger with new EuiToolTip component)

screen shot 2018-03-09 at 3 08 40 pm

@jen-huang jen-huang force-pushed the fix/table-page-size branch from 9354140 to 37bf5ca Compare March 9, 2018 23:20
@elasticmachine
Copy link
Contributor

💔 Build Failed

@chrisronline
Copy link
Contributor

I'm seeing an error when I pull this down and change the field type on the page.

screen shot 2018-03-12 at 9 45 24 am

@jen-huang jen-huang force-pushed the fix/table-page-size branch from 37bf5ca to 06dc13d Compare March 12, 2018 18:30
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jen-huang
Copy link
Contributor Author

@chrisronline Can you update dependencies and check again? Also, upgraded EUI to 0.0.26 instead of 0.0.25 as the newer release fixes some EuiSelect tests.

@chrisronline
Copy link
Contributor

I noticed something else here too. I'm guessing a CSS fix in EUI?

Notice the gray background:
screen shot 2018-03-13 at 9 16 07 am

It's not showing when the tooltip isn't active
screen shot 2018-03-13 at 9 16 13 am

@nreese
Copy link
Contributor

nreese commented Mar 13, 2018

@jen-huang I just created a PR against your PR, jen-huang#1, that will fix the gray background bug found by @chrisronline.

screen shot 2018-03-13 at 8 24 02 am

fix gray background when tooltip opened
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline
Copy link
Contributor

Are we doing this PR or @nreese?

@nreese
Copy link
Contributor

nreese commented Mar 13, 2018

#17110 is merged

@jen-huang
Copy link
Contributor Author

@chrisronline @nreese Merged master to this PR. Now only changes are updating initial number of table rows.

Ok to merge?

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

Sounds good!

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants