Merge pull request #6516 from concourse/issue/5970

Change SideBar "menu" icon
concourse · Feb 12, 2021 · ab76fa8 · ab76fa8
2 parents 35e971a + bf215ae
commit ab76fa8
Show file tree

Hide file tree

Showing 21 changed files with 343 additions and 189 deletions.
diff --git a/web/elm/benchmarks/Benchmarks.elm b/web/elm/benchmarks/Benchmarks.elm
@@ -131,7 +131,7 @@ buildView session model =
         (id "page-including-top-bar" :: Views.Styles.pageIncludingTopBar)
         [ Html.div
             (id "top-bar-app" :: Views.Styles.topBar False)
-            [ SideBar.hamburgerMenu session
+            [ SideBar.sideBarIcon session
             , TopBar.concourseLogo
             , breadcrumbs session model
             , Login.view session.userState model

diff --git a/web/elm/src/Assets.elm b/web/elm/src/Assets.elm
@@ -52,7 +52,10 @@ type Asset
     | DownArrow
     | RefreshIcon
     | MessageIcon
-    | HamburgerMenuIcon
+    | SideBarIconClosedGrey
+    | SideBarIconOpenedGrey
+    | SideBarIconClosedWhite
+    | SideBarIconOpenedWhite
     | PeopleIcon
     | PipelineIconGrey
     | PipelineIconLightGrey
@@ -299,8 +302,17 @@ toPath asset =
         MessageIcon ->
             basePath ++ [ "baseline-message.svg" ]
 
-        HamburgerMenuIcon ->
-            basePath ++ [ "baseline-menu.svg" ]
+        SideBarIconClosedGrey ->
+            basePath ++ [ "baseline-sidebar-closed-grey.svg" ]
+
+        SideBarIconOpenedGrey ->
+            basePath ++ [ "baseline-sidebar-opened-grey.svg" ]
+
+        SideBarIconClosedWhite ->
+            basePath ++ [ "baseline-sidebar-closed-white.svg" ]
+
+        SideBarIconOpenedWhite ->
+            basePath ++ [ "baseline-sidebar-opened-white.svg" ]
 
         PeopleIcon ->
             basePath ++ [ "baseline-people.svg" ]

diff --git a/web/elm/src/Build/Build.elm b/web/elm/src/Build/Build.elm
@@ -640,7 +640,7 @@ view session model =
         (id "page-including-top-bar" :: Views.Styles.pageIncludingTopBar)
         [ Html.div
             (id "top-bar-app" :: Views.Styles.topBar False)
-            [ SideBar.hamburgerMenu session
+            [ SideBar.sideBarIcon session
             , TopBar.concourseLogo
             , breadcrumbs session model
             , Login.view session.userState model

diff --git a/web/elm/src/Colors.elm b/web/elm/src/Colors.elm
@@ -32,7 +32,6 @@ module Colors exposing
     , groupBorderSelected
     , groupBorderUnselected
     , groupsBarBackground
-    , hamburgerClosedBackground
     , infoBarBackground
     , inputOutline
     , instanceGroupBanner
@@ -56,6 +55,7 @@ module Colors exposing
     , sideBarActive
     , sideBarBackground
     , sideBarHovered
+    , sideBarIconBackground
     , sideBarTextBright
     , sideBarTextDim
     , started
@@ -96,8 +96,8 @@ infoBarBackground =
     ColorValues.grey100
 
 
-hamburgerClosedBackground : String
-hamburgerClosedBackground =
+sideBarIconBackground : String
+sideBarIconBackground =
     ColorValues.grey100
 
 

diff --git a/web/elm/src/Dashboard/Dashboard.elm b/web/elm/src/Dashboard/Dashboard.elm
@@ -793,7 +793,7 @@ updateBody session msg ( model, effects ) =
                 Nothing ->
                     ( model, effects )
 
-        Click HamburgerMenu ->
+        Click SideBarIcon ->
             ( model, effects ++ [ GetViewportOf Dashboard ] )
 
         Scrolled scrollState ->
@@ -1009,7 +1009,7 @@ topBar session model =
         (id "top-bar-app" :: Views.Styles.topBar False)
     <|
         [ Html.div [ style "display" "flex", style "align-items" "center" ]
-            [ SideBar.hamburgerMenu session
+            [ SideBar.sideBarIcon session
             , TopBar.concourseLogo
             , TopBar.breadcrumbs session session.route
             ]

diff --git a/web/elm/src/Job/Job.elm b/web/elm/src/Job/Job.elm
@@ -430,7 +430,7 @@ view session model =
         (id "page-including-top-bar" :: Views.Styles.pageIncludingTopBar)
         [ Html.div
             (id "top-bar-app" :: Views.Styles.topBar False)
-            [ SideBar.hamburgerMenu session
+            [ SideBar.sideBarIcon session
             , TopBar.concourseLogo
             , TopBar.breadcrumbs session route
             , Login.view session.userState model

diff --git a/web/elm/src/Message/Effects.elm b/web/elm/src/Message/Effects.elm
@@ -756,6 +756,9 @@ toHtmlID domId =
         StepInitialization stepID ->
             stepID ++ "_image"
 
+        SideBarIcon ->
+            "sidebar-icon"
+
         Dashboard ->
             "dashboard"
 

diff --git a/web/elm/src/Message/Message.elm b/web/elm/src/Message/Message.elm
@@ -95,7 +95,7 @@ type DomID
     | BuildTab Int String
     | JobPreview PipelinesSection Concourse.DatabaseID Concourse.JobName
     | PipelinePreview PipelinesSection Concourse.DatabaseID
-    | HamburgerMenu
+    | SideBarIcon
     | SideBarResizeHandle
     | SideBarTeam PipelinesSection String
     | SideBarPipeline PipelinesSection Concourse.PipelineIdentifier

diff --git a/web/elm/src/NotFound/NotFound.elm b/web/elm/src/NotFound/NotFound.elm
@@ -56,7 +56,7 @@ view session model =
         (id "page-including-top-bar" :: Views.Styles.pageIncludingTopBar)
         [ Html.div
             (id "top-bar-app" :: Views.Styles.topBar False)
-            [ SideBar.hamburgerMenu session
+            [ SideBar.sideBarIcon session
             , TopBar.concourseLogo
             , TopBar.breadcrumbs session model.route
             , Login.view session.userState model

diff --git a/web/elm/src/Pipeline/Pipeline.elm b/web/elm/src/Pipeline/Pipeline.elm
@@ -387,7 +387,7 @@ view session model =
             (id "page-including-top-bar" :: Views.Styles.pageIncludingTopBar)
             [ Html.div
                 (id "top-bar-app" :: Views.Styles.topBar displayPaused)
-                [ SideBar.hamburgerMenu session
+                [ SideBar.sideBarIcon session
                 , TopBar.concourseLogo
                 , TopBar.breadcrumbs session route
                 , PinMenu.viewPinMenu session model

diff --git a/web/elm/src/Resource/Resource.elm b/web/elm/src/Resource/Resource.elm
@@ -944,7 +944,7 @@ view session model =
         (id "page-including-top-bar" :: Views.Styles.pageIncludingTopBar)
         [ Html.div
             (id "top-bar-app" :: Views.Styles.topBar False)
-            [ SideBar.hamburgerMenu session
+            [ SideBar.sideBarIcon session
             , TopBar.concourseLogo
             , TopBar.breadcrumbs session route
             , Login.view session.userState model

diff --git a/web/elm/src/SideBar/SideBar.elm b/web/elm/src/SideBar/SideBar.elm
@@ -2,11 +2,11 @@ module SideBar.SideBar exposing
     ( Model
     , byDatabaseId
     , byPipelineId
-    , hamburgerMenu
     , handleCallback
     , handleDelivery
     , isPipelineVisible
     , lookupPipeline
+    , sideBarIcon
     , tooltip
     , update
     , view
@@ -84,7 +84,7 @@ update message model =
             )
     in
     case message of
-        Click HamburgerMenu ->
+        Click SideBarIcon ->
             let
                 oldState =
                     model.sideBarState
@@ -258,7 +258,17 @@ view model currentPipeline =
 
 
 tooltip : Model m -> Maybe Tooltip.Tooltip
-tooltip { hovered } =
+tooltip model =
+    let
+        hovered =
+            model.hovered
+
+        sideBarState =
+            model.sideBarState
+
+        isSideBarClickable =
+            hasVisiblePipelines model
+    in
     case hovered of
         HoverState.Tooltip (SideBarTeam _ teamName) _ ->
             Just
@@ -299,6 +309,28 @@ tooltip { hovered } =
                 , containerAttrs = Just Styles.tooltipBody
                 }
 
+        HoverState.Tooltip SideBarIcon _ ->
+            let
+                text =
+                    if not isSideBarClickable then
+                        "no visible pipelines"
+
+                    else if sideBarState.isOpen then
+                        "hide sidebar"
+
+                    else
+                        "show sidebar"
+            in
+            Just
+                { body = Html.text text
+                , attachPosition =
+                    { direction = Tooltip.Bottom
+                    , alignment = Tooltip.Middle <| 2 * Styles.tooltipArrowSize
+                    }
+                , arrow = Just 5
+                , containerAttrs = Just Styles.tooltipBody
+                }
+
         _ ->
             Nothing
 
@@ -368,42 +400,54 @@ favoritedPipelinesSection model currentPipeline =
         ]
 
 
-hamburgerMenu : Model m -> Html Message
-hamburgerMenu model =
+sideBarIcon : Model m -> Html Message
+sideBarIcon model =
     if model.screenSize == Mobile then
         Html.text ""
 
     else
         let
-            isHamburgerClickable =
+            isSideBarClickable =
                 hasVisiblePipelines model
+
+            isOpen =
+                model.sideBarState.isOpen
+
+            isHovered =
+                HoverState.isHovered SideBarIcon model.hovered
+
+            assetSideBarIcon =
+                if not isSideBarClickable then
+                    Assets.SideBarIconOpenedGrey
+
+                else if isOpen && isHovered then
+                    Assets.SideBarIconClosedWhite
+
+                else if isOpen && not isHovered then
+                    Assets.SideBarIconClosedGrey
+
+                else if not isOpen && isHovered then
+                    Assets.SideBarIconOpenedWhite
+
+                else
+                    Assets.SideBarIconOpenedGrey
         in
         Html.div
-            (id "hamburger-menu"
-                :: Styles.hamburgerMenu
-                    { isSideBarOpen = model.sideBarState.isOpen && isHamburgerClickable
-                    , isClickable = isHamburgerClickable
-                    }
-                ++ [ onMouseEnter <| Hover <| Just HamburgerMenu
+            (id "sidebar-icon"
+                :: Styles.sideBarMenu isSideBarClickable
+                ++ [ onMouseEnter <| Hover <| Just SideBarIcon
                    , onMouseLeave <| Hover Nothing
                    ]
-                ++ (if isHamburgerClickable then
-                        [ onClick <| Click HamburgerMenu ]
+                ++ (if isSideBarClickable then
+                        [ onClick <| Click SideBarIcon ]
 
                     else
                         []
                    )
             )
             [ Icon.icon
-                { sizePx = 54, image = Assets.HamburgerMenuIcon }
-              <|
-                (Styles.hamburgerIcon <|
-                    { isHovered =
-                        isHamburgerClickable
-                            && HoverState.isHovered HamburgerMenu model.hovered
-                    , isActive = model.sideBarState.isOpen
-                    }
-                )
+                { sizePx = 54, image = assetSideBarIcon }
+                []
             ]
 
 

diff --git a/web/elm/src/SideBar/Styles.elm b/web/elm/src/SideBar/Styles.elm
@@ -5,8 +5,6 @@ module SideBar.Styles exposing
     , SidebarElementColor(..)
     , collapseIcon
     , column
-    , hamburgerIcon
-    , hamburgerMenu
     , iconGroup
     , instanceGroup
     , instanceGroupBadge
@@ -18,6 +16,7 @@ module SideBar.Styles exposing
     , sectionHeader
     , sideBar
     , sideBarHandle
+    , sideBarMenu
     , starPadding
     , starWidth
     , team
@@ -267,10 +266,10 @@ pipelineName { color, weight } =
     ]
 
 
-hamburgerMenu :
-    { isSideBarOpen : Bool, isClickable : Bool }
+sideBarMenu :
+    Bool
     -> List (Html.Attribute msg)
-hamburgerMenu { isSideBarOpen, isClickable } =
+sideBarMenu isClickable =
     [ style "border-right" <| "1px solid " ++ Colors.border
     , style "opacity" "1"
     , style "cursor" <|
@@ -279,23 +278,7 @@ hamburgerMenu { isSideBarOpen, isClickable } =
 
         else
             "default"
-    , style "background-color" <|
-        if isSideBarOpen then
-            Colors.sideBarBackground
-
-        else
-            Colors.hamburgerClosedBackground
-    ]
-
-
-hamburgerIcon : { isHovered : Bool, isActive : Bool } -> List (Html.Attribute msg)
-hamburgerIcon { isHovered, isActive } =
-    [ style "opacity" <|
-        if isActive || isHovered then
-            "1"
-
-        else
-            "0.5"
+    , style "background-color" Colors.sideBarIconBackground
     ]
 
 

diff --git a/web/elm/tests/AssetsTests.elm b/web/elm/tests/AssetsTests.elm
@@ -238,11 +238,26 @@ toStringTests =
                 MessageIcon
                     |> toString
                     |> Expect.equal "/public/images/baseline-message.svg"
-        , test "HamburgerMenuIcon" <|
+        , test "SidebarIconClosedGrey" <|
             \_ ->
-                HamburgerMenuIcon
+                SideBarIconClosedGrey
                     |> toString
-                    |> Expect.equal "/public/images/baseline-menu.svg"
+                    |> Expect.equal "/public/images/baseline-sidebar-closed-grey.svg"
+        , test "SideBarIconOpenedGrey" <|
+            \_ ->
+                SideBarIconOpenedWhite
+                    |> toString
+                    |> Expect.equal "/public/images/baseline-sidebar-opened-white.svg"
+        , test "SidebarIconClosedWhite" <|
+            \_ ->
+                SideBarIconClosedWhite
+                    |> toString
+                    |> Expect.equal "/public/images/baseline-sidebar-closed-white.svg"
+        , test "SideBarIconOpenedWhite" <|
+            \_ ->
+                SideBarIconOpenedWhite
+                    |> toString
+                    |> Expect.equal "/public/images/baseline-sidebar-opened-white.svg"
         , test "PeopleIcon" <|
             \_ ->
                 PeopleIcon

diff --git a/web/elm/tests/DashboardGridTests.elm b/web/elm/tests/DashboardGridTests.elm
@@ -109,7 +109,7 @@ all =
         , test "fetches the viewport of the scrollable area when the sidebar is opened" <|
             \_ ->
                 loadDashboardWithSize 600 600
-                    |> Application.update (Update <| Click HamburgerMenu)
+                    |> Application.update (Update <| Click SideBarIcon)
                     |> Tuple.second
                     |> Common.contains (GetViewportOf Dashboard)
         , test "fetches the viewport of the scrollable area when the sidebar state is loaded" <|