Skip to content

Improve header layout and read-only share UI with status badge, responsive tweaks, and accessibility updates - #110

Merged
alvin000009238 merged 2 commits into
devfrom
codex/reduce-header-height-for-better-ux-w2l7ar
Apr 1, 2026
Merged

Improve header layout and read-only share UI with status badge, responsive tweaks, and accessibility updates#110
alvin000009238 merged 2 commits into
devfrom
codex/reduce-header-height-for-better-ux-w2l7ar

Conversation

@alvin000009238

Copy link
Copy Markdown
Owner

Motivation

  • Replace the ad-hoc DOM-inserted share indicator with a stable header status badge and improve header layout for narrow screens.
  • Improve accessibility of header/time elements and make header controls more compact and consistent across breakpoints.
  • Update static asset references after a build so the HTML loads the correct hashed bundle files.

Description

  • The share link handler now sets the text of a new #headerStatusBadge element instead of inserting a .share-indicator node, and the DOM shows a read-only badge when in share mode.
  • Added .header-status-badge CSS and revised header, logo, data-time-box, and action button styles to reduce padding, tighten gaps, adjust icon/button sizes, and support nowrap/flex behavior.
  • Improved accessibility by adding a visually-hidden time-label, wiring #updateTime to aria-labelledby, adding aria-live to the badge, and adding aria-label attributes to key buttons.
  • Mobile responsive rules updated to wrap header content more cleanly, hide redundant text on small screens, and adjust spacing; several style class names and unused badge styles were removed.
  • Updated public/index.html to include the new badge markup and updated hashed asset filenames for CSS and JS bundles.

Testing

  • Built the frontend with npm run build and verified the generated asset references match the updated index.html, and the build completed successfully.
  • Ran lint and basic CI checks (npm run lint / typecheck) against the changed files and they passed.
  • Performed automated smoke tests to confirm share-link loading path returns handled state and these checks passed.

Codex Task

Copilot AI review requested due to automatic review settings April 1, 2026 03:00

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the header layout and the read-only share indicator, replacing manual DOM manipulation with a dedicated status badge and improving accessibility through ARIA attributes. Feedback was provided regarding a potential layout break on desktop screens when in read-only mode, redundant CSS properties that could affect screen reader accessibility, and visual inconsistencies in button text visibility on mobile devices.

I am having trouble creating individual review comments. Click here to see my feedback.

frontend/styles/compatibility.css (91-94)

medium

Applying width: 100% to .data-time-box globally in read-only mode may disrupt the header layout on desktop screens. Since .header-content is a flex container with nowrap, a 100% width child will compete for space with the .logo element, potentially causing unnecessary shrinking of the logo or layout shifts. This style seems more appropriate for mobile layouts (where it is already defined in responsive.css) and should likely be removed or restricted to a media query here.

frontend/styles/responsive.css (169-171)

medium

The .time-label element is already visually hidden using the sr-only pattern (absolute positioning, 1x1 size, etc.) defined in header.css. Adding display: none here is redundant for layout and can be problematic for accessibility. While aria-labelledby can often still access the text of a display: none element, it is safer and more consistent with the "visually-hidden" approach to let the existing styles handle it, ensuring the label remains available in the accessibility tree for all screen readers.

frontend/styles/responsive.css (190-192)

medium

This rule creates a visual inconsistency on mobile devices. While the text for the share button is hidden to save space (per line 186), the text for the sync button ("載入成績") is explicitly shown here. Since "載入成績" is relatively long and the button already has a descriptive aria-label and icon, it would be more consistent and space-efficient to hide the text for both buttons on small screens.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the share/read-only indicator into a stable header “status badge”, tightens header layout across breakpoints, and updates the built asset references in public/index.html to the latest hashed bundle filenames.

Changes:

  • Replace the DOM-inserted share indicator with a dedicated #headerStatusBadge element updated by the share-link flow.
  • Update header/responsive CSS to reduce padding/gaps, improve truncation/nowrap behavior, and add the new badge styling for read-only mode.
  • Add accessibility attributes/structure for the “update time” label/value, and add aria-labels to key header buttons.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Show a summary per file
File Description
public/index.html Adds header status badge markup + a11y wiring for update time; updates hashed CSS/JS bundle references.
frontend/styles/utilities.css Adjusts theme-toggle button sizing; removes obsolete .readonly-badge styling.
frontend/styles/responsive.css Revises mobile header wrapping/spacing and button label visibility rules.
frontend/styles/header.css Introduces .header-status-badge styling and header layout tightening/truncation updates; makes .time-label visually hidden for a11y.
frontend/styles/compatibility.css Removes legacy .share-indicator styles and adjusts read-only mode header/time layout.
frontend/share.js Updates share-link handling to set #headerStatusBadge text instead of inserting DOM nodes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alvin000009238
alvin000009238 merged commit fe7ebdf into dev Apr 1, 2026
1 check passed
@alvin000009238
alvin000009238 deleted the codex/reduce-header-height-for-better-ux-w2l7ar branch April 1, 2026 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants