Skip to content
This repository has been archived by the owner on May 4, 2022. It is now read-only.

Redesign top bar menu and add side navigation #561

Merged
merged 18 commits into from
Oct 16, 2017

Conversation

thevoiceofzeke
Copy link
Contributor

@thevoiceofzeke thevoiceofzeke commented Oct 3, 2017

This is the result of experimenting with input/request from other groups on campus. Marking it WIP because I want some eyes on it before I spend significant time testing/documenting.

In this PR:

  • Top bar hamburger menu always visible
  • Hamburger menu button triggers side navigation
  • Side nav closes when clicking outside or when scrolling
  • Side navigation is configurable via APP_OPTIONS.appMenuItems
  • Side navigation subheader (above the options) uses the configurable NAMES.title (aka the app title) to differentiate it from portal.theme.title (aka MyUW/uPortal)
  • On mobile, side navigation overlays top bar and includes username menu, notification button, and features button (if applicable)
  • Replace unused directive with mainMenu
  • If no custom menu items are configured, side nav only displays on mobile (with only the basic uportal-app-framework content and footer URLs)
  • Trigger priority notifications from PortalMainController instead of PortalHeaderController, since more things than the header need to know about this (and that controller hierarchy was making things difficult)
  • Includes the changes from fix: Redesign mobile search #560
  • No more crest image. I expect this to be divisive, but I feel strongly that the crest image provides very little benefit while using valuable space and increasing the project's payload.
  • Document how to use new feature

Demos (large/small)

sidenav-large
sidenav-small

Screenshots

screen shot 2017-10-03 at 3 14 58 pm

screen shot 2017-10-03 at 3 17 33 pm

screen shot 2017-10-11 at 11 51 28 am

screen shot 2017-10-03 at 3 17 59 pm

screen shot 2017-10-03 at 3 18 09 pm

screen shot 2017-10-03 at 3 18 18 pm

Without high priority notifications or unseen announcements

screen shot 2017-10-03 at 3 19 35 pm


Contributor License Agreement adherence:

@apetro
Copy link
Contributor

apetro commented Oct 3, 2017

Not gonna lie: I miss the crest. It seems important to branding a portal as institutional, to reassuring that this is "yours", that it's your relationship with the campus. At Madison, it's "MyUW". The "UW" part of that is in part signaled by the crest.

Copy link
Contributor

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @thevoiceofzeke!

@thevoiceofzeke
Copy link
Contributor Author

@apetro I wouldn't be against bringing the crest back, but I think finding an appropriate place to put it is a challenge. The current spot is too much of a blocker of functionality (and we already don't show it on mobile for real estate reasons), and I feel that the benefits of an improved navigation scheme (especially as it concerns developers of other frame apps) outweigh the value of having the crest there.

One immediate change I can include in this PR is showing the crest at the top of the side navigation menu on mobile, so it'll actually have more presence on mobile that it currently does in production.

Would you be able to get on board with considering it a temporary removal, pending some design brainstorming? I agree that it has important branding value and strengthens the app's sense of place, so I'd like to find a way to include it that doesn't block functionality.

@apetro
Copy link
Contributor

apetro commented Oct 4, 2017

I'm already on board, @thevoiceofzeke . I do predict that UX design input, MyUW service team feedback, is going to bring that crest back sooner or later. :) But I'm more than fine with letting the experiment play out, prioritizing developing usable navigation and revisiting branding thereafter. Either the crest comes back in the course of that or it doesn't, and either way will be ok.

@apetro apetro requested a review from mrdahman October 4, 2017 16:06
@cknuth
Copy link

cknuth commented Oct 4, 2017

I'm excited for the side navigation menu! That's something we have struggled with incorporating in STAR and would love to take advantage of.

Rather than a list of links for the navigation, it would be great if we could have a more fine grained control as to what shows in that side menu. For instance, we want to remember search params for some pages in our app which isn't possible with a static link. Furthermore, it would be great to group the links into sections and hide/show those sections based a user's authorities.

If the side nav had an option for a template then we could take matters into our own hands. We've found that we'v been blocked by the limitations of the what we could change in the frame, but having flexibility in these areas would make me more likely to choose the app-framework for IA apps in the future.

@@ -22,6 +22,14 @@ define(['angular'], function(angular) {
.constant('OVERRIDE', {
'APP_OPTIONS': {
'optionsTemplateURL': 'portal/misc/partials/example-options.html',
'appMenuTemplateURL': 'portal/misc/partials/example-menu.html',
Copy link
Contributor

@apetro apetro Oct 4, 2017

Choose a reason for hiding this comment

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

What if the API were a URL that returned JSON representing the menu? Once it's on the other end of a URL, it can be arbitrarily implemented, whether as a static file or as a dynamic back end that considers user identity.

I'd like to avoid escape to an arbitrary template HTML when possible because I'm concerned about uPortal-app-framework ability to provide backwards-compatibility to these templates across upgrades. A JSON API can be cleanly semantically versioned and we can then engineer the framework to continue to support the JSON across versions of the framework, regardless of the evolution of the implementation of the menu.

I think there's a version of this approach that could be successful, where we dig into just what flexibilities need to be designed into the menu. Section headings? Show/hide items depending upon user attributes or group memberships? Icons? Menu items that call JavaScript rather than just hyperlink a URL? By all means, let's, and having defined these needs as an API, then be able to confidently support those usages across framework versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you. This quick and dirty option is meant to meet two of those requirements that have already come up in conversation:

  • Show/hide items depending upon user attributes or group memberships
  • Menu items that call JavaScript rather than just hyperlink a URL

I'm having a hard time envisioning how all those pieces would come together, but I would like to establish a better way of doing this.

</md-menu-content>
</md-menu>
<!-- MAIN MENU TOGGLE -->
<md-button class="menu-toggle" ng-controller="MainMenuController as vm" ng-click="vm.showMainMenu()" ng-class="{ 'hide-gt-xs' : vm.hideMainMenu }">
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this button need some kind of ARIA treatment?

@thevoiceofzeke thevoiceofzeke changed the title [WIP] Redesign top bar menu Redesign top bar menu and add side navigation Oct 11, 2017
Copy link
Contributor

@vertein vertein left a comment

Choose a reason for hiding this comment

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

I too am conflicted about the loss of the crest. I took a look at some other sites and I noticed that quite a few distinguish themselves with a distinctive font.

@thevoiceofzeke
Copy link
Contributor Author

thevoiceofzeke commented Oct 16, 2017

@vertein I've been wondering about embracing UW-Madison's fonts for a while (even if extremely sparingly). It raises some fun questions about theming. Seems like the top bar would be a good place to try it.

@thevoiceofzeke thevoiceofzeke merged commit 222c0a9 into uPortal-Attic:master Oct 16, 2017
@apetro
Copy link
Contributor

apetro commented Oct 16, 2017

This may have introduced #582 .

slight-margin-when-wide

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

Successfully merging this pull request may close these issues.

None yet

5 participants