-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[App Search] Engines Overview polish pass #102778
Conversation
- per Davey's feedback from earlier UI passes
+ update DataPanel icon typing to not error when passed a custom icon/svg - kudos again to Davey for the component
- 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
falls back to a documentation link! so fancy
- 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
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.
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.
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
Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>
Missed updating the heading level
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
* 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>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* 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>
Summary
This PR contains several disparate enhancements to the Engines Overview view. I strongly recommend following along by commit.
Panel cleanup (749b9ea, 62653fb 48c6c49):
<DataPanel />
component Davey created a while backMeta engine license callout (f25efb8, 3bc90b3, 27eadce, f90c3e8)
canManageMetaEngines
capability to include ahasPlatinumLicense
check instead of relying on the data coming in from the ent-search server, which does not update dynamicallyScreencap of CTA + dynamic license behavior:
Dynamic license behavior correctly working on routes:
Checklist