Change SideBar "menu" icon #6516
Conversation
web/elm/src/SideBar/SideBar.elm
Outdated
| HoverState.Tooltip SideBarIcon _ -> | ||
| let | ||
| text = | ||
| if sideBarState.isOpen then |
There was a problem hiding this comment.
Minor thing, but when there are no pipelines (or e.g. you haven't logged in yet and there are no exposed pipelines), you can't actually click on the sidebar icon. Perhaps we shouldn't brighten the icon when you hover over but the icon is not clickable, and perhaps we should give a helpful tooltip (e.g. the sidebar cannot be expanded when there are no pipelines)
Or even weirder
(sidebar was open before logging out/the session expiring, so it displays the hide sidebar icon, but since there are no pipelines the sidebar is hidden)
web/elm/src/SideBar/Styles.elm
Outdated
| hamburgerIcon : { isHovered : Bool, isActive : Bool } -> List (Html.Attribute msg) | ||
| hamburgerIcon { isHovered, isActive } = | ||
| sideBarIcon : Bool -> List (Html.Attribute msg) | ||
| sideBarIcon isHovered = |
There was a problem hiding this comment.
perhaps this should have an isClickable (or just keep the isActive) for the reasons suggested above
- replaced the icon - differentiated between closed and opened sidebar - added tooltips for the button - fixed some tests, we're saving the SideBarFeature tests for a later commit Signed-off-by: Esteban Foronda <eforonda@vmware.com> Co-authored-by: Bohan Chen <bochen@pivotal.io>
- renamed hamburger -> sidebar - added tests for when the sidebar is open/closed - added tests for hover tooltips - replaced the background color tests (since it no longer changes based on if the sidebar is open or not) Signed-off-by: Esteban Foronda <eforonda@vmware.com> Co-authored-by: Bohan Chen <bochen@pivotal.io>
- stopped using opacity and instead use 4 separate svg for open/closed + hovered/unhovered states - update tests Signed-off-by: Esteban Foronda <eforonda@vmware.com> Co-authored-by: Bohan Chen <bochen@pivotal.io>
There was a problem hiding this comment.
Looks good! Just need to fix yarn benchmark now (web/elm/benchmarks/Benchmarks.elm): https://ci.concourse-ci.org/builds/86742463#L6022bb40:45:48
- Adding no pipeline tooltip message - Keeping sidebar grey when is not clickable - Adding no pipeline tooltip message test - Updating tests - Updating benchmarks Signed-off-by: Esteban Foronda <eforonda@vmware.com> Co-authored-by: Bohan Chen <bochen@pivotal.io>
clarafu
added
the
release/undocumented
What does this PR accomplish?
Bug Fix | Feature | Documentation
closes #5970
Blocked on #6453 since it uses the new
Tooltip.BottomalignmentChanges proposed by this PR:
Update the icon and add tooltips
Notes to reviewer:
Release Note
Contributor Checklist
Reviewer Checklist
BOSH and
Helm packaging; otherwise, ignored for
the integration
tests
(for example, if they are Garden configs that are not displayed in the
--helptext).