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

[Design] Branding changes in Elastic to focus more towards the Elastic brand #58160

Merged
merged 17 commits into from
Mar 10, 2020
Merged

[Design] Branding changes in Elastic to focus more towards the Elastic brand #58160

merged 17 commits into from
Mar 10, 2020

Conversation

snide
Copy link
Contributor

@snide snide commented Feb 20, 2020 •
edited
Loading

Fixes #59172
Fixes #59170
Fixes #59167
Fixes #59160
Fixes #59165

Summary

Branding changes in Elastic to focus more towards the Elastic brand.

Animation is burpy because Kibana loads everything at once

Newsfeed cleaned up (more than just branding)

image

Login page

image

Add data removes Kibana

image

@snide snide added Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v8.0.0 v7.7.0 labels Feb 20, 2020
@snide snide requested review from a team as code owners February 20, 2020 18:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-design (Team:Design)

@shaunmcgough
Copy link

@snide this is G-R-E-A-T. Swami says bigger!

@pgayvallet
Copy link
Contributor

But this is an initial PoC just to show what files to edit.

please ping the platform team back when the PR is ready for review 😄

@snide snide mentioned this pull request Mar 3, 2020
Closed
5 tasks
@snide snide requested a review from a team March 4, 2020 00:58
@snide snide changed the title [DRAFT] Loading branding [Design] Branding changes in Elastic to focus more towards the Elastic brand Mar 4, 2020
@snide
Copy link
Contributor Author

snide commented Mar 4, 2020 •
edited
Loading

PR is now up to date.

@gchaps Screens for you to edit. "What's new AT Elastic" for the news feed felt the most natural to me?

@elastic/kibana-platform The burpy animation is not due to it being expensive. Kibana seems to go through quite the loading the process in the background. The network load is pretty wild. Anything you can do in the short term to help us out? Seems weird not to have a loader that animates. For what it's worth, it looks like this view gets loading twice somehow.

@elastic/kibana-pm Could still use some direction on what you want to see from the tabs. Are we switching the fav icon?

@snide snide added the release_note:skip Skip the PR/issue when compiling release notes label Mar 4, 2020
@snide snide requested a review from ryankeairns March 4, 2020 01:18
@pgayvallet
Copy link
Contributor

@elastic/kibana-platform The burpy animation is not due to it being expensive. Kibana seems to go through quite the loading the process in the background. The network load is pretty wild. Anything you can do in the short term to help us out? Seems weird not to have a loader that animates.

The network load shouldn't be an issue for the renderer thread. However all the js bundles are parsed during this loading page (and this do block the main thread), and we indeed are loading a lot of js code. I'm not sure there is any quick win for this though, at the role of this page is to actually load them. @joshdover ?

For what it's worth, it looks like this view gets loading twice somehow.

WDYM by that?

@joshdover
Copy link
Contributor

I'm not aware of any way we can make this any better until we remove some of these huge JS files when we remove support for legacy plugins (#56205).

If there is a way to force the browser to do parsing and compilation off the main thread, that would help, but I haven't found any way to do this. I will pull this PR down though and see if there isn't some easy opportunity to make this faster.

ryankeairns
ryankeairns reviewed Mar 4, 2020
View reviewed changes
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Took this for a spin and found the following:

  • The logo atop login form looks good
  • "Welcome to Elastic Kibana" on login screen looks good
  • Main tab title looks good - "Elastic Kibana"

Things to consider:

  • While the main title tab looks good, there are several apps that still need to be updated. Not sure how we want to handle that (e.g. Canvas, SIEM, Timelion, Infra, etc.)
  • Presuming Josh doesn't have any quick wins, I think we should go back to the old animation and just swap the Elastic logo for the Kibana logo inside of the white circle.
  • Noticed the difference on the "What's New" flyout, we'll see what Gail has to say re: "at" vs "from"

Thanks for knocking these out!

@joshdover
Copy link
Contributor

I looked at profile of this in Chrome Dev Tools and it does appear that the animation frame drops significantly while Chrome is compiling and evaluating all the JS code. I don't know of any ways to make that better except to ship less code. You may want to test a real build as it may be better (we minify the JS shipped to the browser heavily, it's about 1/2 as large as in dev mode).

It looks like this animation is currently implemented using a CSS keyframe animation. From my understanding, these generally have good performance and are usually GPU accelerated. That said, it's possible we would get better performance out of a WebGL animation. I don't have any experience here, maybe someone on the new DataVis team could help.

All that said, it might be worth forgoing the animation until we remove legacy plugins. I suspect we're going to be able to load a lot less code up front once we get there.

@flash1293
Copy link
Contributor

flash1293 commented Mar 4, 2020 •
edited
Loading

I didn't test this and I have no idea how credible this is, but this old blog post indicates transform-only animations are done off-ui-thread (the test page in the blog works for me on desktop chrome). It looks like the loading animation in this PR is animating opacity as well, might be worth a shot trying to only use transforms.

@joshdover did you already try this?

@snide
Copy link
Contributor Author

snide commented Mar 4, 2020

The animation isn't the issue, but don't worry, I understand the problems on the platform side. We'll revert, and use something that still burps, but burps in a less annoying way.

Current idea is to just use an indeterminate loading bar under a static logo.

Thanks for giving it a shot. NP or bust.

@snide
Copy link
Contributor Author

snide commented Mar 4, 2020

Hilariously rereading that article I forgot the simplest solution might just be using an animated gif :)

@gchaps
Copy link
Contributor

gchaps commented Mar 5, 2020

@snide @ryankeairns "What's new at Elastic" reads best.

@ryankeairns ryankeairns self-requested a review March 9, 2020 18:32
ryankeairns
ryankeairns approved these changes Mar 9, 2020
View reviewed changes
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

LGTM

timroes
timroes approved these changes Mar 10, 2020
View reviewed changes
Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Kibana App Code LGTM

@alexfrancoeur
Copy link

I cleaned out cache and loaded up this PR and noticed that the splash screen is still using the Kibana logo (the login screen has the Elastic cluster). As part of #59170, I tried to call out the splash screen but maybe should have opened another issue. Think we can update?

image

@snide
Copy link
Contributor Author

snide commented Mar 10, 2020 via email

@snide
Copy link
Contributor Author

snide commented Mar 10, 2020

@elastic/kibana-platform and @elastic/kibana-security

These are superficial changes to login pages. No functionality has changed. Do you want to give this a quick review?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

legrego
legrego approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

Security changes LGTM

joshdover
joshdover approved these changes Mar 10, 2020
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

LGTM, much smoother animation 😄

@snide snide merged commit 8c0da24 into elastic:master Mar 10, 2020
@snide snide deleted the loading branch March 10, 2020 20:33
@snide snide mentioned this pull request Mar 10, 2020
Merged
@alexfrancoeur
Copy link

❤️

gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 11, 2020
@gmmorris
Merge branch 'master' into alerting/view-in-app
ff30bf8 
* master:
  [ML] Transforms: Use EuiInMemoryTable instead of custom typed table. (elastic#59782)
  Alerting/fix flaky instance test (elastic#58994)
  ci: disable all Mocha rules for tape tests (elastic#59798)
  Fix UX in alerting UI forms when errors occur (elastic#59444)
  [DOCS] Updated and added jump tables (elastic#59774)
  [DOCS] Moved rolled up index content (elastic#59372)
  Regenerate core api docs (elastic#59814)
  [Lens] remove react warnings (elastic#59574)
  The scripts/backport.js file isn't an executable (elastic#59800)
  [Alerting] add more alert properties to action parameter templating (elastic#59718)
  [Design] Branding changes in Elastic to focus more towards the Elastic brand (elastic#58160)
  [SIEM] Adds 'Create new rule' Cypress test (elastic#59790)
  Updating svgo -> css-tree -> mdn-data, all so we get mdn-data > 2.0 (elastic#58913)
  Use EUI test environment build with Jest (elastic#55877)
  update typescript version in all packages to avoid warnings (elastic#59787)
  [SIEM] [Case] Insert timeline into case textarea (elastic#59586)
  [ML] Functional tests - stabilize saved search tests (elastic#59652)
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 11, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

snide added a commit that referenced this pull request Mar 13, 2020
@snide @elasticmachine
[Design] Branding changes in Elastic to focus more towards the Elasti…
3c9bfc5 
…c brand (#58160) (#59827)

Elastic Kibana branding.

Co-authored-by: Elastic Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryankeairns ryankeairns ryankeairns approved these changes

@timroes timroes timroes approved these changes

@joshdover joshdover joshdover approved these changes

@legrego legrego legrego approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone