Merge pull request #608 from dod-ccpo/reskin-nav

Reskin navigation, phase 1

atst/app.py

@@ -22,6 +22,7 @@ from atst.domain.authnid.crl import CRLCache, NoOpCRLCache
 from atst.domain.auth import apply_authentication
 from atst.domain.authz import Authorization
 from atst.domain.csp import make_csp_provider
+from atst.domain.portfolios import Portfolios
 from atst.models.permissions import Permissions
 from atst.eda_client import MockEDAClient
 from atst.utils import mailer
@@ -90,6 +91,14 @@ def make_flask_callbacks(app):
         g.Authorization = Authorization
         g.Permissions = Permissions
 
+    @app.context_processor
+    def _portfolios():
+        if not g.current_user:
+            return {}
+
+        portfolios = Portfolios.for_user(g.current_user)
+        return {"portfolios": portfolios}
+
     @app.after_request
     def _cleanup(response):
         g.current_user = None

atst/routes/__init__.py

@@ -13,6 +13,7 @@ from atst.domain.authnid import AuthenticationContext
 from atst.domain.audit_log import AuditLog
 from atst.domain.auth import logout as _logout
 from atst.domain.common import Paginator
+from atst.domain.portfolios import Portfolios
 from atst.utils.flash import formatted_flash as flash
 
 
@@ -52,16 +53,16 @@ def home():
     if user.atat_role_name == "ccpo":
         return redirect(url_for("requests.requests_index"))
 
-    num_portfolios = len(user.portfolio_roles)
+    num_portfolios = len([role for role in user.portfolio_roles if role.is_active])
 
     if num_portfolios == 0:
         return redirect(url_for("requests.requests_index"))
     elif num_portfolios == 1:
         portfolio_role = user.portfolio_roles[0]
         portfolio_id = portfolio_role.portfolio.id
-        is_request_owner = portfolio_role.role.name == "owner"
+        is_portfolio_owner = portfolio_role.role.name == "owner"
 
-        if is_request_owner:
+        if is_portfolio_owner:
             return redirect(
                 url_for("portfolios.portfolio_reports", portfolio_id=portfolio_id)
             )
@@ -70,7 +71,13 @@ def home():
                 url_for("portfolios.portfolio_applications", portfolio_id=portfolio_id)
             )
     else:
-        return redirect(url_for("portfolios.portfolios"))
+        portfolios = Portfolios.for_user(g.current_user)
+        first_portfolio = sorted(portfolios, key=lambda portfolio: portfolio.name)[0]
+        return redirect(
+            url_for(
+                "portfolios.portfolio_applications", portfolio_id=first_portfolio.id
+            )
+        )
 
 
 @bp.route("/styleguide")

atst/routes/portfolios/__init__.py

@@ -15,14 +15,12 @@ from atst.models.permissions import Permissions
 
 @portfolios_bp.context_processor
 def portfolio():
-    portfolios = Portfolios.for_user(g.current_user)
     portfolio = None
     if "portfolio_id" in http_request.view_args:
         try:
             portfolio = Portfolios.get(
                 g.current_user, http_request.view_args["portfolio_id"]
             )
-            portfolios = [ws for ws in portfolios if not ws.id == portfolio.id]
         except UnauthorizedError:
             pass
 
@@ -33,9 +31,4 @@ def portfolio():
             )
         return False
 
-    return {
+    return {"portfolio": portfolio, "permissions": Permissions, "user_can": user_can}
-        "portfolio": portfolio,
-        "portfolios": portfolios,
-        "permissions": Permissions,
-        "user_can": user_can,
-    }

styles/components/_global_layout.scss

@@ -16,10 +16,6 @@
   flex-grow: 1;
   margin-bottom: $footer-height;
 
-  .global-navigation {
-    margin-top: -1px;
-  }
-
   .global-panel-container {
     margin: $gap;
     max-width: $site-max-width;

styles/components/_topbar.scss

@@ -1,5 +1,5 @@
 .topbar {
-  background-color: $color-white;
+  background-color: $color-blue-darkest;
   border-bottom: 1px solid $color-black;
 
   .topbar__navigation {
@@ -9,7 +9,7 @@
     justify-content: space-between;
 
     .topbar__link {
-      color: $color-black;
+      color: $color-white;
       display: inline-flex;
       align-items: center;
       height: $topbar-height;
@@ -18,10 +18,12 @@
 
       .topbar__link-label {
         @include h5;
+        text-decoration: underline;
       }
 
       .topbar__link-icon {
         margin-left: $gap;
+        @include icon-color($color-white);
       }
 
       &.topbar__link--home {
@@ -29,6 +31,7 @@
 
         .topbar__link-label {
           padding-left: $gap;
+          text-decoration: none;
         }
       }
 
@@ -45,10 +48,6 @@
       &:hover {
         background-color: $color-primary-darker;
         color: $color-white;
-
-        .topbar__link-icon {
-          @include icon-style-inverted;
-        }
       }
     }
 
@@ -62,53 +61,6 @@
       .topbar__portfolio-menu {
         margin-right: auto;
         position: relative;
-
-        .topbar__portfolio-menu__toggle {
-          margin: 0;
-          border-radius: 0;
-
-          &--open {
-            background-color: $color-blue-darkest;
-            position: relative;
-
-            &::before {
-              content: '';
-              display: block;
-              position: absolute;
-              bottom: 0;
-              left: $gap * 2;
-              right: $gap * 2;
-              height: $gap / 2;
-              background-color: $color-primary;
-            }
-          }
-
-          .icon {
-            @include icon-size(10);
-            margin-left: $gap * 2;
-          }
-        }
-
-        .topbar__portfolio-menu__panel {
-          position: absolute;
-        }
-      }
-
-      &.topbar__context--portfolio {
-        background-color: $color-primary;
-        -ms-flex-pack: start;
-
-        .topbar__link {
-          color: $color-white;
-
-          .topbar__link-icon {
-            @include icon-style-inverted;
-          }
-
-          &:hover {
-            background-color: $color-primary-darker;
-          }
-        }
       }
     }
   }

styles/elements/_sidenav.scss

@@ -1,22 +1,54 @@
 .sidenav {
+  @include hide;
+
+  @include media($large-screen) {
+    @include unhide;
+    width: 25rem;
+    margin: 0px;
+  }
+
+  box-shadow: 0 6px 18px 0 rgba(48,58,65,0.15);
+
+  .sidenav__title {
+    color: $color-gray-dark;
+    padding: $gap ($gap * 2);
+    text-transform: uppercase;
+    opacity: 0.54;
+    font-size: $small-font-size;
+    font-weight: bold;
+  }
+
   ul {
+    &.sidenav__list--padded {
+      margin: 4 * $gap 0;
+    }
+
     list-style: none;
-    margin: 0;
     padding: 0;
 
     li {
       margin: 0;
       display: block;
     }
+
+  }
+
+  .sidenav__divider--small {
+    display: block;
+    width: 4 * $gap;
+    border: 1px solid #D6D7D9;
+    margin-left: 2 * $gap;
+    margin-bottom: $gap;
   }
 
   .sidenav__link {
     display: block;
-    border-top: 1px solid $color-black;
     padding: $gap ($gap * 2);
     color: $color-black;
     text-decoration: none;
     white-space: nowrap;
+    overflow: hidden;
+    text-overflow: ellipsis;
 
     .sidenav__link-icon {
       margin-left: - ($gap * .5);
@@ -27,16 +59,33 @@
       pointer-events: none;
     }
 
+    &.sidenav__link--add {
+      color: $color-blue;
+      font-size: $small-font-size;
+      .icon {
+        @include icon-color($color-blue);
+        @include icon-size(14);
+      }
+
+    }
+
     &.sidenav__link--active {
       @include h4;
       color: $color-primary;
-      background-color: $color-white;
+      background-color: $color-aqua-lightest;
       box-shadow: inset ($gap / 2) 0 0 0 $color-primary;
 
       .sidenav__link-icon {
         @include icon-style-active;
       }
 
+      position: relative;
+      .sidenav__link-active_indicator .icon {
+        @include icon-color($color-primary);
+        position: absolute;
+        right: 0;
+      }
+
       + ul {
         background-color: $color-primary;
 
@@ -89,6 +138,7 @@
 
     &:hover {
       color: $color-primary;
+      background-color: $color-aqua-lightest;
 
       .sidenav__link-icon {
         @include icon-style-active;
@@ -97,3 +147,14 @@
     }
   }
 }
+
+.sidenav--minimized {
+  @extend .sidenav;
+
+  @include unhide;
+  margin: 0px;
+
+  @include media($large-screen) {
+    @include hide;
+  }
+}

templates/base.html

@@ -21,7 +21,9 @@
     {% include 'navigation/topbar.html' %}
 
     <div class='global-layout'>
-      {% include 'navigation/global_navigation.html' %}
+      {% block global_sidenav %}
+        {% include 'navigation/global_sidenav.html' %}
+      {% endblock %}
 
       <div class='global-panel-container'>
         {% block sidenav %}{% endblock %}

templates/base_public.html

@@ -17,31 +17,7 @@
 <div id='app-root'>
   {% include 'components/usa_header.html' %}
 
-  <header class="topbar topbar--public">
+  {% include 'navigation/topbar.html' %}
-    <nav class="topbar__navigation">
-      <a href="{{ url_for('atst.home') }}" class="topbar__link topbar__link--home">
-        {{ Icon('shield', classes='topbar__link-icon') }}
-        <span class="topbar__link-label">{{ "base_public.header_title" | translate }}</span>
-      </a>
-
-      {% if g.current_user %}
-        <a href="{{ url_for('users.user') }}" class="topbar__link">
-          <span class="topbar__link-label">{{ g.current_user.first_name + " " + g.current_user.last_name }}</span>
-          {{ Icon('avatar', classes='topbar__link-icon') }}
-        </a>
-
-        <a href="{{ url_for('atst.logout') }}" class="topbar__link" title='Log out of JEDI Cloud'>
-          {{ Icon('logout', classes='topbar__link-icon') }}
-        </a>
-      {% else %}
-        <a href="{{ url_for('atst.home') }}" class="topbar__link" title='Log in'>
-          <span class="topbar__link-label">{{ "base_public.login" | translate }}</span>
-          {{ Icon('avatar', classes='topbar__link-icon') }}
-        </a>
-      {% endif %}
-
-    </nav>
-  </header>
 
   {% block content %}{% endblock %}
 

templates/components/sidenav_item.html

@@ -7,7 +7,14 @@
       {{ Icon(icon, classes="sidenav__link-icon") }}
     {% endif %}
 
-    <span class="sidenav__link-label">{{label}}</span>
+    <span class="sidenav__link-label">
+      {{label}}
+    </span>
+    {% if active %}
+      <span class="sidenav__link-active_indicator">
+        {{ Icon("caret_right") }}
+      </span>
+      {% endif %}
   </a>
 
   {% if subnav and active %}

templates/nav-side.html.to

@@ -1,16 +0,0 @@
-<div class="sidenav">
-  <ul>
-    <li>
-      <a class="sidenav__link" href="/home">Home</a>
-    </li>
-    <li>
-      <a class="sidenav__link" href="/requests">Requests</a>
-    </li>
-  </ul>
-
-  <ul>
-    <li>
-      <a class="sidenav__link" href="/">Logout</a>
-    </li>
-  </ul>
-</div>

templates/navigation/global_navigation.html

@@ -1,19 +0,0 @@
-{% from "components/sidenav_item.html" import SidenavItem %}
-
-<div class="global-navigation sidenav {% if portfolio %}global-navigation__context--portfolio{% endif %}">
-  <ul>
-    {{ SidenavItem("New Task Order",
-      href=url_for("task_orders.get_started"),
-      icon="plus",
-      active=g.matchesPath('/task_orders/new'),
-      ) }}
-
-    {% if g.current_user.has_portfolios %}
-      {{ SidenavItem("Portfolios", href="/portfolios", icon="cloud", active=g.matchesPath('/portfolios')) }}
-    {% endif %}
-
-    {% if g.Authorization.has_atat_permission(g.current_user, g.Permissions.VIEW_AUDIT_LOG) %}
-      {{ SidenavItem("Activity History", url_for('atst.activity_history'), icon="time", active=g.matchesPath('/activity-history')) }}
-    {% endif %}
-  </ul>
-</div>

templates/navigation/global_sidenav.html

@@ -0,0 +1,24 @@
+{% from "components/icon.html" import Icon %}
+{% from "components/sidenav_item.html" import SidenavItem %}
+
+
+<div class="global-navigation sidenav">
+  <div class="sidenav__title">Portfolios</div>
+  <ul class="sidenav__list--padded">
+    {% for other_portfolio in portfolios|sort(attribute='name') %}
+      {{ SidenavItem(other_portfolio.name,
+        href=url_for("portfolios.show_portfolio", portfolio_id=other_portfolio.id),
+        active=(other_portfolio.id | string) == request.view_args.get('portfolio_id')
+        ) }}
+    {% endfor %}
+  </ul>
+  <div class="sidenav__divider--small"></div>
+  <a class="sidenav__link sidenav__link--add" href="{{ url_for("task_orders.get_started") }}" title="Fund a New Portfolio">
+    <span class="sidenav__link-label">Fund a New Portfolio</span>
+    {{ Icon("plus", classes="sidenav__link-icon") }}
+  </a>
+</div>
+
+<div class="global-navigation sidenav--minimized">
+  <div class="sidenav__title">Show >>></div>
+</div>

templates/navigation/topbar.html

@@ -2,71 +2,30 @@
 
 <header class="topbar">
   <nav class="topbar__navigation">
-    {% if not portfolio %}
     <a href="{{ url_for('atst.home') }}" class="topbar__link topbar__link--home">
       {{ Icon('shield', classes='topbar__link-icon') }}
       <span class="topbar__link-label">
         {{ "navigation.topbar.jedi_cloud_link_text" | translate }}
       </span>
     </a>
-    {% else %}
-      <a href="{{ url_for('atst.home') }}" class="topbar__link topbar__link--shield" title="JEDI Cloud Home">
-        {{ Icon('shield', classes='topbar__link-icon') }}
-      </a>
-    {% endif %}
-
-    <div class="topbar__context {% if portfolio %}topbar__context--portfolio{% endif %}">
-      {% if portfolio %}
-
-          <div is='toggler' class='topbar__portfolio-menu'>
-            <template slot-scope='props'>
-              <button
-                v-on:click='props.toggle'
-                class="topbar__link topbar__portfolio-menu__toggle"
-                v-bind:class="{ 'topbar__portfolio-menu__toggle--open': props.isVisible }">
-                <span class="topbar__link-label">{{ "navigation.topbar.named_portfolio" | translate({ "portfolio": portfolio.name }) }}</span>
-                <template v-if='props.isVisible'>{{ Icon('caret_up', classes='topbar__link-icon') }}</template>
-                <template v-else>{{ Icon('caret_down', classes='topbar__link-icon') }}</template>
-              </button>
-
-              <div v-show='props.isVisible' class='topbar__portfolio-menu__panel menu'>
-                <h2 class='menu__heading'>
-                  {{ "navigation.topbar.other_active_portfolios" | translate }}
-                </h2>
-                {% if portfolios %}
-
-                  <ul class='menu__list'>
-                    {% for other_portfolio in portfolios %}
-                      <li class='menu__list__item'>
-                        <a href="{{ url_for('portfolios.show_portfolio', portfolio_id=other_portfolio.id)}}">
-                          {{ other_portfolio.name }}
-                          {{ Icon('caret_right', classes='topbar__link-icon') }}
-                        </a>
-                      </li>
-                    {% endfor %}
-                  </ul>
-
-                {% else %}
-
-                  <p class='menu__message'>
-                    {{ "navigation.topbar.no_other_active_portfolios" | translate }}
-                  </p>
-
-                {% endif %}
-              </div>
-            </template>
-          </div>
-
-      {% endif %}
 
+    <div class="topbar__context">
+      {% if g.current_user %}
         <a href="{{ url_for('users.user') }}" class="topbar__link">
           <span class="topbar__link-label">{{ g.current_user.first_name + " " + g.current_user.last_name }}</span>
           {{ Icon('avatar', classes='topbar__link-icon') }}
         </a>
 
         <a href="{{ url_for('atst.logout') }}" class="topbar__link" title='{{ "navigation.topbar.logout_link_title" | translate }}'>
+          <span class="topbar__link-label">Logout</span>
           {{ Icon('logout', classes='topbar__link-icon') }}
         </a>
+      {% else %}
+        <a href="{{ url_for('atst.home') }}" class="topbar__link" title='Log in'>
+          <span class="topbar__link-label">{{ "base_public.login" | translate }}</span>
+          {{ Icon('avatar', classes='topbar__link-icon') }}
+        </a>
+      {% endif %}
     </div>
   </nav>
 </header>

tests/routes/requests/requests_form/test_new.py

@@ -1,5 +1,6 @@
 import datetime
 import re
+import pytest
 from tests.factories import (
     RequestFactory,
     UserFactory,
@@ -78,6 +79,7 @@ def test_ccpo_can_view_request(client, user_session):
     assert response.status_code == 200
 
 
+@pytest.mark.skip(reason="create request flow no longer active")
 def test_nonexistent_request(client, user_session):
     user_session()
     response = client.get("/requests/new/1/foo", follow_redirects=True)

tests/routes/test_home.py

@@ -2,30 +2,7 @@ import pytest
 
 from tests.factories import UserFactory, PortfolioFactory, RequestFactory
 from atst.domain.portfolios import Portfolios
-
+from atst.models.portfolio_role import Status as PortfolioRoleStatus
-
-def test_user_with_portfolios_has_portfolios_nav(client, user_session):
-    portfolio = PortfolioFactory.create()
-    user_session(portfolio.owner)
-    response = client.get("/home", follow_redirects=True)
-    assert b'href="/portfolios"' in response.data
-
-
-@pytest.mark.skip(reason="this may no longer be accurate")
-def test_user_without_portfolios_has_no_portfolios_nav(client, user_session):
-    user = UserFactory.create()
-    user_session(user)
-    response = client.get("/home", follow_redirects=True)
-    assert b'href="/portfolios"' not in response.data
-
-
-@pytest.mark.skip(reason="this may no longer be accurate")
-def test_request_owner_with_no_portfolios_redirected_to_requests(client, user_session):
-    request = RequestFactory.create()
-    user_session(request.creator)
-    response = client.get("/home", follow_redirects=False)
-
-    assert "/requests" in response.location
 
 
 def test_request_owner_with_one_portfolio_redirected_to_reports(client, user_session):
@@ -51,22 +28,14 @@ def test_request_owner_with_more_than_one_portfolio_redirected_to_portfolios(
     assert "/portfolios" in response.location
 
 
-@pytest.mark.skip(reason="this may no longer be accurate")
-def test_non_owner_user_with_no_portfolios_redirected_to_requests(client, user_session):
-    user = UserFactory.create()
-
-    user_session(user)
-    response = client.get("/home", follow_redirects=False)
-
-    assert "/requests" in response.location
-
-
 def test_non_owner_user_with_one_portfolio_redirected_to_portfolio_applications(
     client, user_session
 ):
     user = UserFactory.create()
     portfolio = PortfolioFactory.create()
-    Portfolios._create_portfolio_role(user, portfolio, "developer")
+    Portfolios._create_portfolio_role(
+        user, portfolio, "developer", status=PortfolioRoleStatus.ACTIVE
+    )
 
     user_session(user)
     response = client.get("/home", follow_redirects=False)
@@ -78,14 +47,20 @@ def test_non_owner_user_with_mulitple_portfolios_redirected_to_portfolios(
     client, user_session
 ):
     user = UserFactory.create()
+    portfolios = []
     for _ in range(3):
         portfolio = PortfolioFactory.create()
-        Portfolios._create_portfolio_role(user, portfolio, "developer")
+        portfolios.append(portfolio)
+        role = Portfolios._create_portfolio_role(
+            user, portfolio, "developer", status=PortfolioRoleStatus.ACTIVE
+        )
 
     user_session(user)
     response = client.get("/home", follow_redirects=False)
 
+    alphabetically_first_portfolio = sorted(portfolios, key=lambda p: p.name)[0]
     assert "/portfolios" in response.location
+    assert str(alphabetically_first_portfolio.id) in response.location
 
 
 @pytest.mark.skip(reason="this may no longer be accurate")

translations.yaml

@@ -290,7 +290,7 @@ login:
   title_tag: Sign in | JEDI Cloud
 navigation:
   topbar:
-    jedi_cloud_link_text: JEDI Cloud
+    jedi_cloud_link_text: JEDI
     logout_link_title: Log out of JEDI Cloud
     named_portfolio: 'Portfolio {portfolio}'
     no_other_active_portfolios: You have no other active JEDI portfolios.