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

Pull down app list #4951

Merged
merged 5 commits into from
Sep 17, 2015
Merged

Conversation

rashidkpc
Copy link
Contributor

Changes the app switcher to function as a pull down container. This removes the need for the user to leave their current context to decide on a new one. It also gets rid of the "Kibana app inside Kibana" issue we had previously. In addition, it makes switching between apps faster since you don't have to wait for the app switcher bundle to load.

  • App switcher is no longer an app itself, rather just a plugin
  • Removed /apps route
  • Implemented the pull down drawer using the standard config directive
  • Moved app list loading from /api/apps into the chrome setup
  • White border removed from kibana barcode. You might want to turn off "rich diff" in the Github diff, as this is a change to an svg, which is always a bunch of messy xml
  • Kibana app always the default route now.
  • Moved app icon to the right, I just like it better

Screenshot:
screen shot 2015-09-15 at 2 38 47 pm

@alt74
Copy link

alt74 commented Sep 16, 2015

kibana 4-2 home menu 1 2x
Tweaked the design a bit. Font Awesome doesn't have a lion icon :( I'm happy to look for a lion icon in the same style as Font Awesome.

@rashidkpc
Copy link
Contributor Author

This makes space for ~6 icons.

  • What happens to the overflow in this design? Does it wrap?
  • The containers appear to be variable width, it will not be possible to get those to align row-to-row. I'd prefer the containers to be static width/height for wrapping predicability.
  • Everything else is in the interface is left aligned, maybe make sense to left align here as well?
  • New shade of gray, do we want that or can we pick an existing shade?

@alt74
Copy link

alt74 commented Sep 17, 2015

kibana 4-2 home menu 2 2x

• I made the assumption we won't have more than 5 in round 1. corrected in this version.
• Switched to existing grays. Round 1 had 1 new gray that went along with the top nav grays. #A3A5AD
• Removed product descriptions. I think the icons help to give the user a hint what the product is about. I think we can live without description.
• Round 1 had menu aligned center to reduce distance from menu button in top nav to products = less cursor traveling. not a big deal. left align is fine.
•  I recommend removing gray behind Kibana logo. When "Discover" is selected the backgrounds are very similar. Looks like an error.

@rashidkpc
Copy link
Contributor Author

Another idea from @alt74

@rashidkpc rashidkpc assigned simianhacker and unassigned alt74 Sep 17, 2015
@simianhacker
Copy link
Member

Codewise... LGTM

As far as the design, I like the text with the subheading. Logos and icons are great but just having the text is even easier for the developer. I kind of wish we had a thingy that defaulted to a text treatment if you didn't provide branding.

@rashidkpc
Copy link
Contributor Author

Decided not do the text only, added auto generated icons for apps without an icon.

screen shot 2015-09-17 at 10 02 56 am

@rashidkpc
Copy link
Contributor Author

And with no icons at all:

screen shot 2015-09-17 at 10 20 15 am

@alt74
Copy link

alt74 commented Sep 17, 2015

👍

@@ -7,5 +7,5 @@ module.exports = _.once(function (kbnServer) {

// redirect to the single app
let apps = kbnServer.uiExports.apps.toArray();
return apps.length === 1 ? `/app/${apps[0].id}` : '/apps';
return '/app/kibana';
Copy link
Contributor

Choose a reason for hiding this comment

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

Created #4968 to make this configurable

@spalger
Copy link
Contributor

spalger commented Sep 17, 2015

LGTM 👍

spalger added a commit that referenced this pull request Sep 17, 2015
@spalger spalger merged commit fef6e70 into elastic:master Sep 17, 2015
@tbragin tbragin mentioned this pull request Sep 21, 2015
8 tasks
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.

4 participants