Skip to content

Unifying converting listing pages to new layout #98651

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
Merged
merged 5 commits into from
May 6, 2021
Merged

Unifying converting listing pages to new layout #98651

merged 5 commits into from
May 6, 2021

Conversation

@myasonik
Copy link
Contributor

@myasonik myasonik commented Apr 28, 2021

Summary

Copy + cleanup of @cchaos's work in her fork (original commit).

Comparisons

Default empty page
Old New
empty default - old empty default - new
Customized empty page
Old New
empty - old empty - new
Listing page
Old New
listing - old listing - new

Checklist

Delete any items that are not applicable to this PR.

@cchaos
adding KibanaPageTemplate and converting listing pages
415f508 
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Simplifying template configuration and updating docs
a838aac 
- Renamed `isEmptyScreen` to `isEmptyState`
- Removed `emptyPrompt` prop in favor of simply passing `in as children`.
- Allowing override of template type when `isEmptyState`
- Updating docs to better match patterns we want as copy/pasteable snippets
This was referenced Apr 29, 2021
Closed
Closed
@myasonik myasonik marked this pull request as ready for review May 3, 2021 15:19
@myasonik myasonik requested a review from a team May 3, 2021 15:19
@myasonik myasonik requested review from a team as code owners May 3, 2021 15:19
@myasonik myasonik added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels May 3, 2021
@myasonik
Copy link
Contributor Author

myasonik commented May 3, 2021

@elasticmachine merge upstream

@flash1293
Copy link
Contributor

flash1293 commented May 4, 2021

It seems like for Graph the white background won't expand to the bottom of the page, while for dashboard it does:
Screenshot 2021-05-04 at 12 13 39

Screenshot 2021-05-04 at 12 14 54

@cchaos
Copy link
Contributor

cchaos commented May 4, 2021

@flash1293 Yep! You'll find a list of Analytic app issues here: #98827. It's as simple as ensuring there's no random <div> wrapper or if there must be, you need to add the APP_WRAPPER_CLASS.

flash1293
flash1293 approved these changes May 4, 2021
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

LGTM in that case. Feel free to add the class name to Graph on this PR, but I can also do this separately.

ThomThomson
ThomThomson approved these changes May 6, 2021
View reviewed changes
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Tested this locally and the new dashboard listing page is great, and works flawlessly with the 'continue editing' list. The new empty screen also works great!

Also took a cursory look through the code. Everything LGTM.

Is it intentional that the dashboard unsaved edits callout shows up under the title, while the Visualize Library 'try dashboard' callout shows up above the title? If this is not intentional, I think they should be aligned since their design is very similar. The Presentation team can most likely do a follow up for this.

Screen Shot 2021-05-06 at 3 57 41 PM

Screen Shot 2021-05-06 at 4 03 21 PM

I suppose Canvas doesn't use the table list view for its listing page, but its design should also be aligned. We'll just need to put in a bit of effort to catch up to the new standard.

Overall, no big problems, and I've been waiting for this redesign ever since I saw the figma!

tsullivan
tsullivan approved these changes May 6, 2021
View reviewed changes
Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

KibanaPageTemplate addition and listing page changes LGTM

@myasonik
Copy link
Contributor Author

myasonik commented May 6, 2021

@elasticmachine merge upstream

@myasonik myasonik added the auto-backport Deprecated - use backport:version if exact versions are needed label May 6, 2021
@kibanamachine
Copy link
Contributor

kibanamachine commented May 6, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
kibanaReact 336 338 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
kibanaReact 207 208 +1

Async chunks

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

id before after diff
dashboard 138.8KB 138.7KB -142.0B
graph 1.1MB 1.1MB -134.0B
kibanaReact 312.0KB 312.0KB +30.0B
maps 2.7MB 2.7MB +122.0B
visualize 91.8KB 91.8KB -4.0B
total -128.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kibanaReact 117.6KB 119.9KB +2.3KB
Unknown metric groups

API count

id before after diff
kibanaReact 232 234 +2

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 4 2 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 5 3 -2
licensing 18 15 -3
monitoring 109 56 -53
total -73

History

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

@myasonik myasonik merged commit 921c942 into master May 6, 2021
@myasonik myasonik deleted the pageLayouts/stage_1/introduce_kbl branch May 6, 2021 22:31
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 6, 2021
@cchaos @kibanamachine
Unifying converting listing pages to new layout (elastic#98651)
2296964 
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-authored-by: cchaos <caroline.horn@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine mentioned this pull request May 6, 2021
Merged
@kibanamachine
Copy link
Contributor

kibanamachine commented May 6, 2021

💚 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 May 7, 2021
@kibanamachine @cchaos
Unifying converting listing pages to new layout (#98651) (#99557)
092995d 
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-authored-by: cchaos <caroline.horn@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Michail Yasonik <michail.yasonik@elastic.co>
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-authored-by: cchaos <caroline.horn@elastic.co>
@stratoula stratoula mentioned this pull request Jun 7, 2021
Closed
@nreese nreese mentioned this pull request Aug 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tsullivan tsullivan tsullivan approved these changes

@flash1293 flash1293 flash1293 approved these changes

@ThomThomson ThomThomson ThomThomson approved these changes

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 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@myasonik @flash1293 @cchaos @kibanamachine @tsullivan @ThomThomson