Compact header height for better above-the-fold density - #101
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refines the header UI by adjusting dimensions, spacing, and layout properties across base and responsive stylesheets. Key changes include reducing the size of the logo and action buttons, updating padding and gaps, and modifying the responsive behavior of the header content. Feedback identifies potential content overflow risks introduced by disabling flex wrapping and points out several redundant CSS declarations in the responsive styles that are already inherited from the base configuration.
| align-items: center; | ||
|
|
||
| flex-wrap: wrap; | ||
| flex-wrap: nowrap; |
There was a problem hiding this comment.
將 .header-content 設置為 flex-wrap: nowrap 雖然能確保高度固定，但在內容較多（例如標題文字較長或導航項目增加）的情況下，可能會導致內容溢出容器。建議為標題區域設置 min-width: 0 並對 h1 使用 text-overflow: ellipsis，以確保在空間不足時能優雅處理。
| flex-direction: row; | ||
|
|
||
| align-items: center; | ||
|
|
||
| gap: 10px; | ||
|
|
||
| align-items: flex-start; | ||
| flex-wrap: nowrap; |
There was a problem hiding this comment.
在此媒體查詢中，align-items: center 與 flex-wrap: nowrap 已在 header.css 的基礎樣式中定義，此處重複設置是多餘的。此外，在手機版強制單行顯示 (flex-direction: row 且 nowrap) 存在佈局溢出風險，建議僅保留必要的屬性變更並檢查窄螢幕下的表現。
| flex-direction: row; | |
| align-items: center; | |
| gap: 10px; | |
| align-items: flex-start; | |
| flex-wrap: nowrap; | |
| flex-direction: row; | |
| gap: 10px; |
| font-size: 18px; | ||
|
|
||
| line-height: 1.2; |
There was a problem hiding this comment.
此處的 line-height: 1.2 與 header.css 第 101 行中設置的基礎樣式完全相同。由於該屬性會被繼承且未被修改，在媒體查詢中重複定義是多餘的。
| font-size: 18px; | |
| line-height: 1.2; | |
| font-size: 18px; |
There was a problem hiding this comment.
Pull request overview
This PR reduces the visual height of the top header to improve above-the-fold content density (especially on mobile) by tightening spacing and resizing header elements, while aiming to preserve usable tap targets.
Changes:
- Reduced header vertical padding and tightened header layout spacing (gap, nowrap behavior).
- Reduced logo/icon sizing and normalized text vertical metrics (line-height, margins).
- Reduced interactive control sizing (theme toggle button 40→36, primary header buttons use min-height 36 with smaller horizontal padding) and adjusted mobile header layout (single-row, hide time-info).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/styles/header.css | Compacts header padding/layout, reduces logo sizing, adjusts typography metrics, and shrinks primary header button sizing. |
| frontend/styles/responsive.css | Updates <=768px header layout to stay in a compact single-row configuration and hides .time-info on mobile. |
| frontend/styles/utilities.css | Shrinks .theme-toggle-btn dimensions from 40×40 to 36×36. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| align-items: center; | ||
|
|
||
| flex-wrap: wrap; | ||
| flex-wrap: nowrap; | ||
|
|
||
| gap: 16px; | ||
| gap: 12px; |
There was a problem hiding this comment.
Changing .header-content to flex-wrap: nowrap makes the header unable to wrap when content is wider than the viewport (logo + 2 action buttons + optional share indicator). Since body sets overflow-x: hidden (frontend/styles/base.css:33-46), this will clip header controls on narrow screens rather than allowing them to remain accessible. Consider allowing wrapping below a small breakpoint, or forcing the logo/text area to shrink/truncate (e.g., min-width: 0 + ellipsis) so the action buttons always fit.
| gap: 10px; | ||
|
|
||
| align-items: flex-start; | ||
| flex-wrap: nowrap; |
There was a problem hiding this comment.
In the mobile breakpoint, forcing .header-content to stay single-row and nowrap increases the likelihood that the header contents overflow on small devices (e.g., 320px width with "匯入成績" button, plus the readonly share indicator). With overflow-x: hidden on body, this will cut off controls instead of reflowing them. Consider adding an additional narrower breakpoint to either allow wrapping, hide button labels, or apply truncation/shrink rules so all controls remain reachable.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .header-content { | ||
|
|
||
| flex-direction: column; | ||
| flex-wrap: wrap; | ||
|
|
||
| align-items: flex-start; | ||
| gap: 8px; |
There was a problem hiding this comment.
In the 768px breakpoint the header is allowed to wrap (flex-wrap: wrap), which can still introduce multi-line headers and reduce above-the-fold density. This also conflicts with the PR description that aims to keep the header single-row; consider keeping flex-wrap: nowrap here and relying on min-width: 0 + ellipsis (and/or hiding secondary elements) to prevent height growth.
|
|
||
| .header-actions { | ||
| width: auto; | ||
| width: 100%; |
There was a problem hiding this comment.
Setting .header-actions { width: 100%; } at the 768px breakpoint forces the actions onto a new line inside .data-time-box, increasing header height on small screens. If the goal is a more compact header, keep actions inline (auto width) and instead reduce spacing/labels or hide less-important elements.
| width: 100%; | |
| width: auto; |
| .time-info { | ||
| padding-right: 8px; | ||
| } |
There was a problem hiding this comment.
The PR description mentions hiding secondary time-info on small screens, but the 480px rules still keep .time-info visible (only padding is adjusted). If the intent is to remove it to save space, hide the .time-info container (and remove its right border) rather than only tweaking padding.
| } | ||
|
|
||
| .header-actions .import-dropdown-btn > span:last-child { | ||
| display: none; |
There was a problem hiding this comment.
On <=480px the visible text label inside .import-dropdown-btn is removed with display: none, but the affected buttons (e.g. 分享 / 匯入成績) do not have aria-labels in the markup. This leaves icon-only controls without an accessible name for screen readers; add aria-label/title to the buttons or hide the label visually (not with display:none).
| display: none; | |
| position: absolute; | |
| width: 1px; | |
| height: 1px; | |
| padding: 0; | |
| margin: -1px; | |
| overflow: hidden; | |
| clip: rect(0, 0, 0, 0); | |
| white-space: nowrap; | |
| border: 0; |
| flex-wrap: wrap; | ||
| } | ||
|
|
There was a problem hiding this comment.
.header-actions is now allowed to wrap by default, which can increase header height at intermediate widths (even outside mobile breakpoints) and partially defeats the goal of a compact header. Consider keeping it nowrap by default and only enabling wrapping at the smallest breakpoint(s) where it’s strictly necessary.
| flex-wrap: wrap; | |
| } | |
| flex-wrap: nowrap; | |
| } | |
| @media (max-width: 600px) { | |
| .header-actions { | |
| flex-wrap: wrap; | |
| } | |
| } |
Motivation
Description
.header垂直 padding 從14px調整為10px，並把.header-content改為nowrap並縮小 gap 以避免換行撐高容器。.logo-icon36→32、svg 20→18）並為標題/副標題設定line-height與移除h1預設 margin，修正文字造成的額外高度。min-height: 36px並減少水平 padding，主題切換按鈕改為36x36，以維持至少 36px 的最小點擊區域而不鎖定容器高度。.header-content維持為單行（row）、縮小間距、隱藏次要的time-info，讓 header 在小螢幕更緊湊。Testing
npm test（前端測試），所有測試通過（13 tests, 0 failed）。npm run build成功產生 production build 並更新靜態資產。Codex Task