[ZEPPELIN-1002] Move configuration menus under dropdown

[minahlee]

What is this PR for?

• Move configuration menus under dropdown menu
• Change dropdown menu style
• Change Login button style

What type of PR is it?

Improvement

Todos

• Fix selenium test

What is the Jira issue?

ZEPPELIN-1002

Screenshots (if appropriate)

As anonymous

Before

After

With shiro authc

Before

After

### Questions: - Does the licenses files need update? No - Is there breaking changes for older versions? No - Does this needs documentation? It needs image updates

[corneadoug]

I think this kind of menu should be under a setting icon instead.
And keep the admin dropdown element only for logout if there is auth

[Leemoonsoo]

Thanks @minahlee for the update. I think it's less confusing and more organized way to manage menubar. Do you think it'll be merged into branch-0.6, too?

On documentation, I think there're some screenshots related to this update.
for example this, this, and so on. Could you handle them here or create an issue for it?

[minahlee]

I think it is common practice to have settings under account dropdown menu.

Examples

Github

Trello

UI Candidates for managing setting menus

which one do you prefer among below two?

• Have separator dividing settings menu
  

• Have separate icon for settings

  

In addition to the change I've already made, I am going to put username under menu to take care of long user name. This is current UI which seems not pleasant experience for users:

@Leemoonsoo yes I think it's possible to push this into branch-0.6 if we decide how we want to display it.

[bzz]

Just FYI - please mind that second menu should play nice with mobile layout as well.

As first one is collapsed on mobile, even right now with just a search - it already sometimes not enough space there.

I love the idea of reducing number of elements in nav menu, but why do not we just keep 2 elements in the main navigation: "Notebooks" and "Everything else" (->About, Interpreters, etc).

This way we do not need to think about mobile, etc as existing Bootstrap setup will take care of that. What do you think?

[corneadoug]

@bzz actually, last time I tested, the about dropdown was doing great on mobile

@minahlee I still prefer separated one, mainly because you won't always have a user, and
having a dropdown under Connected/Disconnected is misleading and doesn't give much indications.
The thing about settings is that it describes well what you will find inside: settings

In the case of website separating them, we can list JIRA and Gmail:

For the content for both menu, it would be best to have a better separation in their role: What is under user (if there is one), should be related to the user: Logout. And what is under setting should be related to the instance/software: Interpreter, Credentials, Configuration, About

[bzz]

@bzz actually, last time I tested, the about dropdown was doing great on mobile

Have you already tested 2 proposed dropdowns on mobile? If possible, plz share the screenshots

[minahlee]

@corneadoug Having separate menu for the anonymous user totally makes sense.
Then the next question would be where do we want to place it?
Next to Notebook? - will give less confusion who is used to current layout
On the right side of nav bar? - will take less space since it will use gear icon instead of Settings text.

I have no special preference on setting menu location.
@corneadoug, @bzz, @Leemoonsoo what is your opinion here?

[Leemoonsoo]

'Credentials' is per user. So if we want to divide 'user' and 'setting', it'll be

user - Credentials, Logout
setting - Interpreter, Configuration, About

Or how about simply create a button,

o Admin (gear icon)

and put every items under this single button?
Either way, i think it's better than now.

[corneadoug]

@bzz Here is a screenshot of example showing a second dropdown, technically its 3 dropdown if you count the Notebooks one

@mina I think a setting icon would be better as it doesn't take much space in the navbar
Style in mobile can always be modified to be more verbose in different PR. (I already found a few bugs)

@Leemoonsoo What is the Credential menu btw?

[Leemoonsoo]

@corneadoug Credential menu, see https://issues.apache.org/jira/browse/ZEPPELIN-828

[corneadoug]

Maybe we can even rename that menu: 'My credentials' then :)

[Leemoonsoo]

If 'Credential' menu is under 'User', i think it already imply 'My credential'. isn't it?

[minahlee]

One question, if Confidential is per user menu, should it be shown when only user logged in?

[Leemoonsoo]

I think 'Credential' still useful without login. It just becomes (anonymous user's) credential.

[cloverhearts]

Confidential default user id value is "anonymous"

[bzz]

@corneadoug thanks for posting screen! It looks reasonable

and put every items under this single button?
Either way, i think it's better than now.

True, as soon as it works on mobile as well and Integration tests are updated there is not that much difference and we can change it again later.

[minahlee]

Thanks for the feedback guys! I've just pushed some commits and updated description with new screenshot.

[corneadoug]

For the username inside the dropdown, is it not possible to have the long username tooltip disappear if the menu is out? Except for that LGTM

[minahlee]

What about showing user name under dropdown only when username length exceeds MAX_USERNAME_LENGTH?

[corneadoug]

@minahlee Actually, the long name would normally be truncated by CSS instead of javascript, so it wouldn't be good. (Since it was done elsewhere, no need to fix it here)

Is it necessary to show long name though? Maybe this can be handled in a different PR, so that we can think of other way of doing it, and not block this one.

[minahlee]

I also don't feel the necessity of showing full user name since logged user would know his own username. Let me remove username under dropdown then.

[bzz]

Thank you @minahlee for updating the screenshots!

But now we have 3 drop-down menus :) Are you guys sure that we can not get away with just 2 of them i.e 'logout' instead of 'about' in under the gear icon?

Also, for the sake of consistency, should not it be "Anonymous" instead of "Connected" near the green\red circle when the user is not logged in?

[corneadoug]

@bzz In the case of Anonymous, I would really prefer keeping anything to do with User Management out of Zeppelin if it's not using Auth. Since there is one single user, there is no need for Anonymous anywhere.

[bzz]

Ok, thanks for explanation, this was not a strong opinion, just a question. Sounds good to me.
@minahlee what do you think?

[minahlee]

I am ok with either approach, seems like @corneadoug has strong opinion on having two separate menu so I will leave it as it is. I will need to fix selenium test to make it to be merged though.

[minahlee]

Updated selenium test and CI passes

[corneadoug]

@minahlee You removed the truncated text, but didn't implement it in css.
Could you add a class with this inside to this element?

white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
max-width: 180px;

[corneadoug]

LGTM

[Leemoonsoo]

CI failure is irrelevant to this change.
Tested and it looks good to me. +1 for merge it into master and branch-0.6

[minahlee]

Merging it into master and branch-0.6

[khalidhuseynov]

I've just went through the conversation here, and i think that having userName instead of connected status in authenticated mode with additional logout menu is quite misleading. first of all connected and disconnected serve different purpose and not related to logged in user, so they have to be separated. Secondly, i would be expecting most of the action features like logout under main dropdown menu, and wouldn't expect to click on my name to see logout button. So maybe need to rethink it a bit :)

[minahlee]

@khalidhuseynov Thanks for the feedback.
I totally agree that showing Connected message for non-logged user can be confusing.
I made several changes based on your opinion.

• show user name as anonymous when shiro is configured as anonymous mode.(suggested by @bzz in previous comment)
• move websocket connected/disconnected text to tooltip.
• merge two dropdown menu to one. I am also skeptical about having one more dropdown menu only for logout action. I believe with caret icon, users will easily know where to click if they wants to do some action like change settings, logout, etc.

anonymous

authc

[AhyoungRyu]

@khalidhuseynov Right. I totally agree with you.
@minahlee It's more intuitive than before 👍

[corneadoug]

I agree with @khalidhuseynov that having Connected/Disconnected being replace by the username isn't great.

However I still think that showing a username cf: 'anonymous' for an app without any Auth system activated doesn't make any sense.

Regarding not having two dropdowns, I don't mind grouping it in one, or having a separate icon next to the username for logout.

[Leemoonsoo]

I think showing 'anonymous' without auth system activated still make sense.
All authorization features (i.e. notebook permission, credential) works normally as user name 'anonymous' when auth system is not activated.

So, showing 'anonymous' is actually helping, i think.

[khalidhuseynov]

@minahlee thanks for quick mockup! looks good and as @corneadoug said we can omit anonymous when no one logged in, and only show username if authentication is enabled. One more thing is about tooltip, not sure whether we need to give details of WebSocket there and whether it's going to be helpful, that's just an opinion :)

[minahlee]

Thanks for the feedback all!
@Leemoonsoo After your explanation, it makes sense to display anonymous
@khalidhuseynov I added WebSocket because logged user can misunderstand that he is connected/disconnected to session.
Integration test passes, I will merge this both to master, branch-0.6 if there is no more discussion

[khalidhuseynov]

LGTM