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.

Sign up for GitHub

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

Jump to bottom

[App Search] Engines Overview polish pass #102778

Merged
merged 9 commits into from
Jun 21, 2021
Merged

[App Search] Engines Overview polish pass #102778

merged 9 commits into from
Jun 21, 2021

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Jun 21, 2021 •
edited
Loading

Summary

This PR contains several disparate enhancements to the Engines Overview view. I strongly recommend following along by commit.

Panel cleanup (749b9ea, 62653fb 48c6c49):

  • Converts the Engines section and Meta Engines sections into separate panels, per @daveyholler's feedback a few days ago
  • DRY's out the various manual EUI flex groups etc to reuse the <DataPanel /> component Davey created a while back
  • Updates the DataPanel to accept a custom title size + other small typography tweaks

Meta engine license callout (f25efb8, 3bc90b3, 27eadce, f90c3e8)

  • Creates a "Manage license" button/link that intelligently falls back to a documentation link if the user does not have permission to manage licenses (+ updates Enterprise Search Overview/home to use said button)
  • Adds a new Meta Engine license upgrade CTA, that is always visible if the account does not have a platinum license
  • Tweaks our canManageMetaEngines capability to include a hasPlatinumLicense check instead of relying on the data coming in from the ent-search server, which does not update dynamically

Screencap of CTA + dynamic license behavior:

meta_engines_cta

Dynamic license behavior correctly working on routes:

create_meta_engine

Checklist

@cee-chen
Split up engines vs. meta engines into separate panels
749b9ea 
- per Davey's feedback from earlier UI passes
@cee-chen
DRY out manual header/spacing to reusable DataPanel component
62653fb 
+ update DataPanel icon typing to not error when passed a custom icon/svg

- kudos again to Davey for the component
@cee-chen
Typography tweaks
48c6c49 
- Update DataPanel component to accept a custom titleSize (to maintain previous UI/sizing)

- Fix meta engines empty prompt title heading to follow heading levels + tweak sizing to not be larger than panel heading
@cee-chen
Set up new license CTA button for upcoming meta engines CTA
f25efb8 
falls back to a documentation link! so fancy
@cee-chen
Update Enterprise Search Overview to use new license button
3bc90b3
@cee-chen
Add new Meta Engines license upgrade CTA
27eadce 
- Reuse some copy from Meta Engines creation view
- Reuse DataPanel so visuals stay consistent + it looks similar to CTA on Enterprise Search Overview
- Update DataPanel to allow buttons to be responsive + conditionally load spacer between header & children
@cee-chen
Improve responsiveness of app when platinum license changes
f90c3e8 
Previously, routes/apps were going off the static data passed from the server which was only initialized once on page load. hasPlatinumLicense however changes dynamically and in real-time, removing the need for a hard page refresh.

I could have replaced all `canManageMetaEngine` flags with `isPlatinum && canManageEngines`, but I thought baking license checks into the main ability would be more scalable and potentially open the way to other license-based flags also being dynamic.
@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 21, 2021
@cee-chen cee-chen requested review from daveyholler, JasonStoltz and a team June 21, 2021 17:20
JasonStoltz
JasonStoltz approved these changes Jun 21, 2021
View reviewed changes
Copy link
Member

@JasonStoltz JasonStoltz left a comment

Choose a reason for hiding this comment

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

LGTM, a couple of little nits

...nterprise_search/public/applications/app_search/components/engines/engines_overview.test.tsx Outdated Show resolved Hide resolved
...s/enterprise_search/public/applications/app_search/components/data_panel/data_panel.test.tsx Outdated Show resolved Hide resolved
Constance and others added 2 commits June 21, 2021 11:43
@JasonStoltz
[PR feedback] Typos in test names
0f74026 
Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>
Fix failing test
49ed5ad 
Missed updating the heading level
@cee-chen cee-chen enabled auto-merge (squash) June 21, 2021 19:13
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1453 1454 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.1MB 2.1MB +3.1KB

History

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

@cee-chen cee-chen merged commit 6dc9962 into elastic:master Jun 21, 2021
@cee-chen cee-chen deleted the engines-overview-cleanup branch June 21, 2021 20:52
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 21, 2021
@JasonStoltz @kibanamachine
[App Search] Engines Overview polish pass (elastic#102778)
50a2385 
* Split up engines vs. meta engines into separate panels

- per Davey's feedback from earlier UI passes

* DRY out manual header/spacing to reusable DataPanel component

+ update DataPanel icon typing to not error when passed a custom icon/svg

- kudos again to Davey for the component

* Typography tweaks

- Update DataPanel component to accept a custom titleSize (to maintain previous UI/sizing)

- Fix meta engines empty prompt title heading to follow heading levels + tweak sizing to not be larger than panel heading

* Set up new license CTA button for upcoming meta engines CTA

falls back to a documentation link! so fancy

* Update Enterprise Search Overview to use new license button

* Add new Meta Engines license upgrade CTA

- Reuse some copy from Meta Engines creation view
- Reuse DataPanel so visuals stay consistent + it looks similar to CTA on Enterprise Search Overview
- Update DataPanel to allow buttons to be responsive + conditionally load spacer between header & children

* Improve responsiveness of app when platinum license changes

Previously, routes/apps were going off the static data passed from the server which was only initialized once on page load. hasPlatinumLicense however changes dynamically and in real-time, removing the need for a hard page refresh.

I could have replaced all `canManageMetaEngine` flags with `isPlatinum && canManageEngines`, but I thought baking license checks into the main ability would be more scalable and potentially open the way to other license-based flags also being dynamic.

* [PR feedback] Typos in test names

Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>

* Fix failing test

Missed updating the heading level

Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>
@kibanamachine kibanamachine mentioned this pull request Jun 21, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 21, 2021
@kibanamachine @JasonStoltz
[App Search] Engines Overview polish pass (#102778) (#102819)
ed92036 
* Split up engines vs. meta engines into separate panels

- per Davey's feedback from earlier UI passes

* DRY out manual header/spacing to reusable DataPanel component

+ update DataPanel icon typing to not error when passed a custom icon/svg

- kudos again to Davey for the component

* Typography tweaks

- Update DataPanel component to accept a custom titleSize (to maintain previous UI/sizing)

- Fix meta engines empty prompt title heading to follow heading levels + tweak sizing to not be larger than panel heading

* Set up new license CTA button for upcoming meta engines CTA

falls back to a documentation link! so fancy

* Update Enterprise Search Overview to use new license button

* Add new Meta Engines license upgrade CTA

- Reuse some copy from Meta Engines creation view
- Reuse DataPanel so visuals stay consistent + it looks similar to CTA on Enterprise Search Overview
- Update DataPanel to allow buttons to be responsive + conditionally load spacer between header & children

* Improve responsiveness of app when platinum license changes

Previously, routes/apps were going off the static data passed from the server which was only initialized once on page load. hasPlatinumLicense however changes dynamically and in real-time, removing the need for a hard page refresh.

I could have replaced all `canManageMetaEngine` flags with `isPlatinum && canManageEngines`, but I thought baking license checks into the main ability would be more scalable and potentially open the way to other license-based flags also being dynamic.

* [PR feedback] Typos in test names

Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>

* Fix failing test

Missed updating the heading level

Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JasonStoltz JasonStoltz JasonStoltz approved these changes

@daveyholler daveyholler Awaiting requested review from daveyholler

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.14.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cee-chen @kibanamachine @JasonStoltz