Skip to content

[CCR] Migrate to new page layout structure#102507

Merged
sabarasaba merged 7 commits intoelastic:masterfrom
sabarasaba:ccr/new_layout
Jun 22, 2021
Merged

[CCR] Migrate to new page layout structure#102507
sabarasaba merged 7 commits intoelastic:masterfrom
sabarasaba:ccr/new_layout

Conversation

@sabarasaba
Copy link
Copy Markdown
Member

@sabarasaba sabarasaba commented Jun 17, 2021 •
edited
Loading

This PR migrates Ingest Cross Cluster Replication to use the new page layout. Also did a few small UX improvements:

  • Moved the create cta's next to the search box of each table in the main page
  • Fixed the flyout manage pop-over padding

Related: #100748

Screenshots

Follower Indices Home

Screenshot 2021-06-21 at 12 11 36
Screenshot 2021-06-21 at 12 16 05
Screenshot 2021-06-21 at 12 16 56
Screenshot 2021-06-21 at 12 17 13
Screenshot 2021-06-21 at 12 17 31

Auto-follow patterns

Screenshot 2021-06-21 at 12 20 02
Screenshot 2021-06-21 at 12 12 21
Screenshot 2021-06-21 at 12 19 26
Screenshot 2021-06-21 at 12 19 43
Screenshot 2021-06-21 at 12 21 11

Follower Indices create

Screenshot 2021-06-21 at 12 25 52
Screenshot 2021-06-21 at 12 26 10

Follower Indices edit

Screenshot 2021-06-21 at 12 27 26
Screenshot 2021-06-21 at 12 26 41

Auto-follow pattern create

Screenshot 2021-06-21 at 12 28 38
Screenshot 2021-06-21 at 12 28 49

Auto-follow pattern edit

Screenshot 2021-06-21 at 12 29 38
Screenshot 2021-06-21 at 12 29 46

App Permissions

Screenshot 2021-06-21 at 12 13 11
Screenshot 2021-06-21 at 12 13 38
Screenshot 2021-06-21 at 12 14 14

Fixed popover padding

Before
Screenshot 2021-06-21 at 12 08 48

After
Screenshot 2021-06-21 at 12 09 06

@sabarasaba sabarasaba self-assigned this Jun 17, 2021
@sabarasaba sabarasaba mentioned this pull request Jun 17, 2021
Closed
10 tasks
@sabarasaba sabarasaba added chore Feature:CCR and Remote Clusters release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.14.0 v8.0.0 labels Jun 21, 2021
@sabarasaba sabarasaba marked this pull request as ready for review June 21, 2021 12:13
@sabarasaba sabarasaba requested a review from a team as a code owner June 21, 2021 12:13
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 21, 2021

Pinging @elastic/kibana-stack-management (Team:Stack Management)

@sabarasaba
Copy link
Copy Markdown
Member Author

sabarasaba commented Jun 21, 2021

@elasticmachine merge upstream

@cjcenizal
Copy link
Copy Markdown
Contributor

cjcenizal commented Jun 21, 2021

@sabarasaba I chatted with Caroline last week about whether to center or top-align states under tabs, and she advised to consistently center these states. You can see some examples of what I mean in the screenshots on #101548. It ends up simplifying things in some ways (fewer components to use) but also complecting things in others (having to use APP_WRAPPER_CLASS and sometimes nested EuiPageContent instances in creative ways to get states to center but tables to top-align.

I think it makes sense to defer these changes until after this PR is merged, to ensure we have the baseline improvements merged before feature freeze. Once this is merged, would you mind taking another pass to apply the above guidance?

CC @alisonelizabeth

@sabarasaba
Copy link
Copy Markdown
Member Author

sabarasaba commented Jun 21, 2021

@cjcenizal Sounds like a plan! I can certainly go through it after we merge it and patch it up so that they are all centered. I can also do the same on the snapshot+restore and upgrade assistant apps which I believe also have their states top-aligned as well 🚀

@alisonelizabeth
Copy link
Copy Markdown
Contributor

alisonelizabeth commented Jun 21, 2021

@sabarasaba I chatted with Caroline last week about whether to center or top-align states under tabs, and she advised to consistently center these states.

👍 Thanks for the heads up @cjcenizal!

I can also do the same on the snapshot+restore and upgrade assistant apps which I believe also have their states top-aligned as well 🚀

@sabarasaba If you can take care of Snapshot + Restore that'd be great! Let's hold off on UA. While there are currently tabs, they will eventually be going away (likely during the 7.15 cycle). Since UA will be disabled for 7.14, I don't think it makes sense to address this now.

alisonelizabeth
alisonelizabeth approved these changes Jun 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Great work @sabarasaba!

The only thing I had a question about is the loading state on the add/edit screens.

Screen Shot 2021-06-21 at 2 25 31 PM

I believe in similar situations in some of the apps I migrated, I made this a full-page loading state with no page header. It seemed to flow better IMO since the permissions loading state appears first and is displayed in this way. Let me know what you think. /cc @cchaos

@alisonelizabeth
Copy link
Copy Markdown
Contributor

alisonelizabeth commented Jun 21, 2021

Also, thanks for fixing the popover! I think that regression is related to the new EUI theme (#97699).

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented Jun 21, 2021

I believe in similar situations in some of the apps I migrated, I made this a full-page loading state with no page header. It seemed to flow better IMO since the permissions loading state appears first and is displayed in this way. Let me know what you think.

Yeah I think if it's possible that after loading, the page header would be replaced by a full empty/error state, then make the loading state it's own as well.

@sabarasaba
Copy link
Copy Markdown
Member Author

sabarasaba commented Jun 22, 2021

Thanks for the suggestion @alisonelizabeth! Just pushed 2 more commits to address that, lmkwyt

Screenshots

Screenshot 2021-06-22 at 13 50 56
Screenshot 2021-06-22 at 13 51 21
Screenshot 2021-06-22 at 14 12 51

@alisonelizabeth
Copy link
Copy Markdown
Contributor

alisonelizabeth commented Jun 22, 2021

Thanks @sabarasaba! I did not retest, but screenshots LGTM.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 22, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
crossClusterReplication 294.2KB 290.5KB -3.7KB

History

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

cc @sabarasaba

@sabarasaba sabarasaba merged commit 69a5d01 into elastic:master Jun 22, 2021
sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Jun 22, 2021
@sabarasaba @kibanamachine
[CCR] Migrate to new page layout structure (elastic#102507)
cbaa686 
* wip: start migrating views from ccr

* finish up migrating ccr pages to new nav layout

* Fix tests, linter errors and i18n strings

* remove todo

* Render loading and error states centered in screen without page title

* Keep loader going while we still setting the payload

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@sabarasaba sabarasaba mentioned this pull request Jun 22, 2021
Merged
sabarasaba added a commit that referenced this pull request Jun 22, 2021
@sabarasaba @kibanamachine
[CCR] Migrate to new page layout structure (#102507) (#102946)
cc051ce 
* wip: start migrating views from ccr

* finish up migrating ccr pages to new nav layout

* Fix tests, linter errors and i18n strings

* remove todo

* Render loading and error states centered in screen without page title

* Keep loader going while we still setting the payload

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@sabarasaba sabarasaba mentioned this pull request Jun 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alisonelizabeth alisonelizabeth alisonelizabeth approved these changes

Assignees

@sabarasaba sabarasaba

Labels

chore Feature:CCR and Remote Clusters release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.14.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sabarasaba @elasticmachine @cjcenizal @alisonelizabeth @cchaos @kibanamachine