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.

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

[7.0] Adds new K7 side navigation #28940

Merged
merged 26 commits into from
Jan 31, 2019

Conversation

ryankeairns
Copy link
Contributor

@ryankeairns ryankeairns commented Jan 17, 2019

Closes #25736

Summary

Adds new side navigation to K7 design (see gif below), thus replacing the current app drawer button in the current K7 header. The side nav is always visible, expands upon hover, and overlays plugin/page content as opposed to pushing it over.

One notable addition is the placement of 'Recent activity' atop the navigation making it more easily accessible. Future plans include potentially adding search, pinning, and favorites to this new navigation.

There has been discussion of grouping a few 'Admin' apps together into a flyout similar to 'Recent activity', but more discussion is needed on the technical side.

Lastly, the Uptime app is awaiting a new icon, so you'll see an empty slot in the collapsed navigation.

Demo

nav-demo

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

To-do items

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ryankeairns
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ryankeairns ryankeairns requested a review from snide January 17, 2019 19:27
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

1. I'm still seeing the K logo section wider than the nav drawer:

screen shot 2019-01-17 at 14 31 40 pm

2. Thoughts on allowing wrapping of recent items?

Also, was there any discussion over showing the app icon as well?

screen shot 2019-01-17 at 14 37 40 pm

3. I found a bug where clicking on Graph keeps redirecting to the homepage

I'm guessing this has to do with the license of the user and it's somehow not getting hidden for the basic license.

4. Speaking of recent, can we go ahead and drop the recently viewed section on the homepage?

screen shot 2019-01-17 at 14 41 10 pm

@ryankeairns
Copy link
Contributor Author

ryankeairns commented Jan 17, 2019

@cchaos

1. I'm still seeing the K logo section wider than the nav drawer:

This will be fixed in the next build of EUI, these changes occurred after 6.4.0

2. Thoughts on allowing wrapping of recent items?

Also, was there any discussion over showing the app icon as well?

I need to talk about this with Spencer. Upon a quick look, I don't see any values in the recent activity data that we could use to determine the source app other than the url, but that would be messy. I suspect we'd need to somehow capture this info at the time the object is viewed and stored.

3. I found a bug where clicking on Graph keeps redirecting to the homepage

I'm guessing this has to do with the license of the user and it's somehow not getting hidden for the basic license.

That makes sense! There is a 'hidden' value in the link data, but I it's not being handled in terms of actually hiding the link. I'll dig into this.

4. Speaking of recent, can we go ahead and drop the recently viewed section on the homepage?

Good question. The new homepage does display the recent items (in the keypad/icon list), so maybe we should keep it here for now. @alexf thoughts on this?

@cchaos
Copy link
Contributor

cchaos commented Jan 17, 2019

2. Thoughts on allowing wrapping of recent items?

These item names can be quite long, but we can make it a flag that only recent items can wrap text but the app links can't.

@snide
Copy link
Contributor

snide commented Jan 17, 2019

  1. Speaking of recent, can we go ahead and drop the recently viewed section on the homepage?

We can drop it. It's duplicative at this point. Anything we can do to unclutter that already crowded screen is a good idea in my book.

@ryankeairns ryankeairns removed the WIP Work in progress label Jan 17, 2019
@ryankeairns ryankeairns changed the title [WIP] [7.0] Adds new K7 side navigation [7.0] Adds new K7 side navigation Jan 17, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alexfrancoeur
Copy link

Looking good! Just took the PR for a spin, some feedback from my end.

Where'd we land with moving dev console, management, stack monitoring, etc. into an admin subgroup? I have a scroll on my macbook today, it'd be nice to avoid if possible. I believe (but am not certain) that code search will also be available in 7.0 but need to find out soon. @AlonaNadler @yaronp68 thoughts?

image

For recently viewed, I think we need to find a way to better showcase long names. In the homepage today, on hover you'll get the full string. What can we do to improve this experience?

image

Looks like we are no longer persisting the side bar if the window shrinks. At what size does this happen? And are we OK with this icon?

image

This hover affect seems just a little to slow to react in my opinion. I think this is going to be key to the experience so would really like others feedback to tweak it right.

jan-18-2019 14-04-29

Also, I think ordering will be important here. Did we end up removing the different "sections"?

image

Great work @ryankeairns!!

@cjcenizal
Copy link
Contributor

cjcenizal commented Jan 18, 2019

Took a quick look. This is looking really great! I agree with Alex about the animation though. As a user, I just want to get from X to Y, and I'm going to need to do it often. In this case, I think less is more -- the less the animation gets in my face, the less distracting it is, and the more quickly I can use the navigation. A few suggestions come to mind for making the animation feel snappier:

  1. Removing the fade-in from each item will make then feel more responsive.
  2. I really think the transition-delay should be dropped.
  3. You could try removing the animation altogether, so the drawer just snaps open and shut.
  4. If you really want to keep a flourish, maybe try snapping it open with a slight bounce back?

@ryankeairns
Copy link
Contributor Author

ryankeairns commented Jan 18, 2019

@alexfrancoeur

  • There is a change coming on the EUI side to make the long title under 'Recently viewed' wrap.
  • While the EUI component can handle the sections, we don't have currently have an available flag in the recent links data to differentiate those. There are ways to make that work, but I need to chat with an engineer about how best to approach that.
  • The drawer hides on 575px and down, which is narrower than a tablet (e.g. iPad) when in portrait mode. Additional breakpoints can be added, but we are comfortable with this for now given the current state of mobile-readiness for the apps themselves.
  • I'll play around with the current animations settings and alternatives shared by @cjcenizal

Thank you both for your feedback!

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@spalger spalger mentioned this pull request Jan 30, 2019
15 tasks
@cjcenizal
Copy link
Contributor

Thanks @ryankeairns and @snide!

@spalger spalger mentioned this pull request Jan 30, 2019
@spalger
Copy link
Contributor

spalger commented Jan 31, 2019

I think the chrome should be hidden on the welcome screen

image

@elasticmachine

This comment has been minimized.

@spalger
Copy link
Contributor

spalger commented Jan 31, 2019

@ryankeairns @cchaos I added the icons to the recently accessed items by matching them to navLinks the same way we do for "last url" handling

@spalger
Copy link
Contributor

spalger commented Jan 31, 2019

Tooltips that are close to the nav go underneath it, but maybe not all of them? Maybe we can get them to overlay the chrome even though that might look a little Escher due of the drop shadows.

image

@ryankeairns
Copy link
Contributor Author

@spalger Thanks for pointing those out and continuing to chip away at this PR. I’ll take a look at both, though they may need to be follow up PRs just given time (and that we’ll have a longer QA period).

I’ll poke around tomorrow and see what these might entail. One way or another, we’ll address them. Thanks!

@elasticmachine

This comment has been minimized.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Just giving the OK for this, with the understanding we have some follow up issues around:

  1. locking the minimized state
  2. dealing with the login chrome
  3. trying to think up somthing for tooltips. my guess is the container pattern will help here but we'll be SOL for non EUI tooltips, which is ok.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

SHIP IT! 😄

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants