feat(header): Chevron right for collapsed - chevron down for expanded#4318
feat(header): Chevron right for collapsed - chevron down for expanded#4318Simpler1 wants to merge 1 commit intoZoneMinder:masterfrom
Conversation
|
@IgorA100 opinion on this? |
To be honest, after this PR it becomes even more unclear :) As an option - maybe replace with collapse_all + expand_all |
There was a problem hiding this comment.
Pull request overview
Updates the classic skin header toggle affordance so the chevron direction reflects the collapsed/expanded state of the secondary navbar (“navbar-two”), and persists that state via a cookie.
Changes:
- Switched the toggle icon behavior to use right chevron for collapsed and down chevron for expanded.
- Renamed persisted cookie values from
up/downtoshown/hidden. - Updated PHP rendering logic to hide/show
#navbar-twoand set the initial chevron based on the cookie.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
web/skins/classic/js/skin.js |
Changes the toggle logic to swap keyboard_arrow_right/keyboard_arrow_down and writes `zmHeaderFlip=hidden |
web/skins/classic/includes/functions.php |
Updates initial navbar visibility and the rendered chevron based on the new cookie values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $header = ( isset($_COOKIE['zmHeaderFlip']) and $_COOKIE['zmHeaderFlip'] == 'shown') ? 'down' : 'right'; | ||
| $result .= '<li id="getHeaderFlipHTML" class="nav-item dropdown"><a class="nav-link" href="#"><i id="flip" class="material-icons md-18" style="display: inline-block;">keyboard_arrow_' .$header. '</i></a></li>'.PHP_EOL; |
There was a problem hiding this comment.
getHeaderFlipHTML() defaults the chevron to right unless the cookie is explicitly 'shown'. With no cookie set (first visit) the navbar is rendered visible, but the icon will indicate the collapsed state. Consider defaulting to down when the navbar is visible (e.g., when cookie is not 'hidden'), and also mapping legacy values (up/down) appropriately.
| function navbarTwoFlip() { | ||
| $j("#navbar-two").slideToggle("slow"); | ||
| const flip = $j("#flip"); | ||
| if ( flip.html() == 'keyboard_arrow_up' ) { | ||
| flip.html('keyboard_arrow_down'); | ||
| setCookie('zmHeaderFlip', 'down'); | ||
| if ( flip.html() == 'keyboard_arrow_down' ) { | ||
| flip.html('keyboard_arrow_right'); | ||
| setCookie('zmHeaderFlip', 'hidden'); | ||
| } else { | ||
| flip.html('keyboard_arrow_up'); | ||
| setCookie('zmHeaderFlip', 'up'); | ||
| flip.html('keyboard_arrow_down'); | ||
| setCookie('zmHeaderFlip', 'shown'); |
There was a problem hiding this comment.
navbarTwoFlip() decides which icon/cookie to set based on the current icon text (flip.html()). If a user arrives with no zmHeaderFlip cookie (or legacy values like up/down), the server-rendered icon/state can be out of sync with the actual #navbar-two visibility, causing the toggle to persist the wrong cookie value and display the wrong chevron. Consider basing the logic on #navbar-two's actual visibility (before toggling) and/or treating legacy cookie values (down => hidden, up/unset => shown) as aliases when computing the next state.
|
|
||
| <nav class="navbar navbar-expand-md justify-content-center" id="navbar-two" | ||
| <?php echo ( isset($_COOKIE['zmHeaderFlip']) and $_COOKIE['zmHeaderFlip'] == 'down' ) ? 'style="display:none;"' : '' ?> | ||
| <?php echo ( isset($_COOKIE['zmHeaderFlip']) and $_COOKIE['zmHeaderFlip'] == 'hidden' ) ? 'style="display:none;"' : '' ?> |
There was a problem hiding this comment.
The new cookie value check only hides #navbar-two when zmHeaderFlip == 'hidden'. Users with an existing cookie from previous versions (down was used to indicate the hidden state) will now get the navbar shown unexpectedly and can end up with icon/state mismatches. Consider treating legacy values (e.g., down) as equivalent to hidden, or migrating/clearing the cookie.
| <?php echo ( isset($_COOKIE['zmHeaderFlip']) and $_COOKIE['zmHeaderFlip'] == 'hidden' ) ? 'style="display:none;"' : '' ?> | |
| <?php echo ( isset($_COOKIE['zmHeaderFlip']) && ( $_COOKIE['zmHeaderFlip'] == 'hidden' || $_COOKIE['zmHeaderFlip'] == 'down' ) ) ? 'style="display:none;"' : '' ?> |
Simple change to make the chevrons easier to understand when the main-header-nav is collapsed or expanded.
The right chevron indicates collapsed.
The down chevron indicates expanded.
(Reference: https://stackoverflow.com/questions/18325779/bootstrap-3-collapse-show-state-with-chevron-icon)