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

[KibanaPageLayout] Solution Nav specific styles & props #100089

Merged
merged 18 commits into from
May 25, 2021
Merged

[KibanaPageLayout] Solution Nav specific styles & props #100089

merged 18 commits into from
May 25, 2021

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented May 13, 2021 •
edited
Loading

Added solutionNav prop to populate EuiSideNav and create an embellished navigation title

See Docs Tutorial section for details.

kibana_template_solution_nav

Including mobile version

Screen Recording 2021-05-14 at 11 41 10 AM

Checklist

  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
    • Ensures all page content is contained by landmarks is caused by the <h2> not being contained in the <nav> element. But they are hooked up via aria-labelledby. So for now, it's ok. If we want to fix this properly, we'll have to change the way EuiSideNav renders.
  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
  • This was checked for cross-browser compatibility

@cchaos cchaos mentioned this pull request May 13, 2021
Closed
9 tasks
@cchaos cchaos requested a review from myasonik May 14, 2021 17:39
@cchaos cchaos added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels May 14, 2021
@cchaos cchaos mentioned this pull request May 14, 2021
Closed
22 tasks
myasonik
myasonik approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

A few small things but otherwise LGTM

dev_docs/tutorials/kibana_page_template.mdx Outdated Show resolved Hide resolved
dev_docs/tutorials/kibana_page_template.mdx Outdated Show resolved Hide resolved
src/plugins/kibana_react/public/page_template/page_template.tsx Outdated Show resolved Hide resolved
src/plugins/kibana_react/public/page_template/solution_nav/solution_nav.scss Show resolved Hide resolved
@cchaos cchaos marked this pull request as ready for review May 14, 2021 20:01
@cchaos cchaos requested review from a team as code owners May 14, 2021 20:01
@cchaos
Copy link
Contributor Author

cchaos commented May 18, 2021

@elasticmachine merge upstream

@cchaos cchaos requested a review from a team as a code owner May 19, 2021 17:42
@cchaos
Copy link
Contributor Author

cchaos commented May 19, 2021

@elastic/kibana-operations , I used the --update-limits command to increase the limit for kibanaReact because of the additional CSS added in this PR.

Screen Shot 2021-05-19 at 13 39 20 PM

Screen Shot 2021-05-19 at 13 39 07 PM

jbudz
jbudz approved these changes May 19, 2021
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

limits.yml LGTM

@spalger
Copy link
Contributor

spalger commented May 19, 2021 •
edited
Loading

@elasticmachine merge upstream

Confirmed by diffing the bundles that the only change in core.entry.js is the media query CSS.

@cchaos
Copy link
Contributor Author

cchaos commented May 25, 2021

@elasticmachine merge upstream

@mattkime
Copy link
Contributor

@cchaos Is there anything I should do to test this? As best I can tell, the implementation isn't consumed anywhere.

@cchaos
Copy link
Contributor Author

cchaos commented May 25, 2021

@mattkime Correct, it isn't consumed anywhere, yet. If you want to test it, you can change the render of table_list_view.tsx to this: https://gist.github.com/cchaos/e0a9b461c4ba58847c03c13fd8a85d47

mattkime
mattkime approved these changes May 25, 2021
View reviewed changes
Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

changes lgtm, work well

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
kibanaReact 338 367 +29

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 219 218 -1

Async chunks

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

id before after diff
kibanaReact 312.0KB 312.0KB +4.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
kibanaReact 3 4 +1

Page load bundle

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

id before after diff
core 400.8KB 408.0KB +7.2KB
kibanaReact 119.9KB 133.7KB +13.8KB
total +21.0KB
Unknown metric groups

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 22 20 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 261 149 -112
lens 67 45 -22
licensing 18 15 -3
lists 239 236 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
securitySolution 386 342 -44
stackAlerts 101 95 -6
total -342

History

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

@cchaos cchaos merged commit bca1c14 into elastic:master May 25, 2021
@cchaos cchaos mentioned this pull request May 25, 2021
Merged
cchaos added a commit to cchaos/kibana that referenced this pull request May 25, 2021
@cchaos
[KibanaPageLayout] Solution Nav specific styles & props (elastic#100089)
43ecbfa 
* Fixing sticky nav
* Adding some side bar styles
* Added a built-in solution nav title with avatar icon
* Adding tutorial docs
* Added KibanaPageTemplateSolutionNavAvatar
* Added KibanaPageTemplateSolutionNav
* Increased limit to `core` / `kibanaReact` plugin because of additional CSS
# Conflicts:
#	packages/kbn-optimizer/limits.yml
@cchaos cchaos deleted the pageLayouts/stage_1/solution_nav branch May 25, 2021 17:39
@weltenwort weltenwort mentioned this pull request May 25, 2021
Merged
24 tasks
cchaos added a commit that referenced this pull request May 25, 2021
@cchaos
[KibanaPageLayout] Solution Nav specific styles & props (#100089) (#1…
f07d18c 
…00576)

* Fixing sticky nav
* Adding some side bar styles
* Added a built-in solution nav title with avatar icon
* Adding tutorial docs
* Added KibanaPageTemplateSolutionNavAvatar
* Added KibanaPageTemplateSolutionNav
* Increased limit to `core` / `kibanaReact` plugin because of additional CSS
# Conflicts:
#	packages/kbn-optimizer/limits.yml
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 26, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into reporting/ua-…
812f82c 
…deprecation-ilm-policy

* 'master' of github.com:elastic/kibana: (101 commits)
  [ftr] migrate "docTable" service to FtrService class (elastic#100595)
  [ftr] migrate "listingTable" service to FtrService class (elastic#100606)
  Fixed comparing real value with formatted according to mode. (elastic#100456)
  [ftr] migrate "dataGrid" service to FtrService class (elastic#100593)
  [ftr] migrate "fieldEditor" to FtrService class (elastic#100597)
  [ftr] migrate "filterBar" service to FtrService class (elastic#100601)
  [triggersActionsUi] Reduce page load bundle to under 100kB (elastic#97770)
  [build] Clean jest configs (elastic#100594)
  refact(NA): remove extra pkg_npm target and add specific target folders for @kbn/analytics on Bazel (elastic#100569)
  Update dependency @elastic/charts to v29.2.0 (elastic#100587)
  [Maps] convert LayerPanel to typescript (elastic#100481)
  [Upgrade Assistant] Address copy feedback (elastic#99632)
  Open/Closed filter for observability alerts page (elastic#99217)
  One liner to expose the EQL query for debugging for users (elastic#100565)
  [KibanaPageLayout] Solution Nav specific styles & props (elastic#100089)
  [ftr] implement FtrService classes and migrate common services (elastic#99546)
  [XY] [Lens] Adds opacity slider (elastic#100453)
  [Reporting] ILM policy for managing reporting indices (elastic#100130)
  [Reporting] ILM policy for managing reporting indices (elastic#100130)
  [DOCS] Remove redundant maps attribute (elastic#100426)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/lib/store/report_ilm_policy.ts
#	x-pack/plugins/reporting/server/lib/store/store.test.ts
#	x-pack/plugins/reporting/server/lib/store/store.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@myasonik myasonik myasonik approved these changes

@jbudz jbudz jbudz approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

@mattkime mattkime mattkime approved these changes

Assignees
No one assigned
Labels
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.
7 participants
@cchaos @spalger @mattkime @kibanamachine @ryankeairns @jbudz @myasonik