Skip to content

style.css declares .nav-btn and .ch-icon-btn at both 48px and 44px #2052

Description

@efiten

Both selectors carry two conflicting minimum-size declarations in public/style.css. The later one wins, so 44px is what ships while the earlier block claims 48px.

selector first declaration later declaration effective
.nav-btn :561 min-height/min-width: 48px :890 min-width: 44px; min-height: 44px 44
.ch-icon-btn :567 min-height/min-width: 48px :1838 min-width: 44px; min-height: 44px 44

The two blocks also disagree in their reasoning. The 48px block at :610 says "Same 48x48 minimums + touch-action so all interactive surfaces meet WCAG 2.5.5", while the 44px rule at :1836 says "WCAG 2.5.5 / Apple HIG: 44x44 CSS px minimum touch target". WCAG 2.5.5 is 44x44, so the later comment cites it correctly and the earlier one over-claims: 48 is the Material figure, not the WCAG one.

Nothing is visually broken today, and I am not proposing a size change: which number the project wants is a design call, not a lint. What is wrong is that the stylesheet states both, so a reader cannot tell what the rule is and a future edit to either block silently changes behaviour or silently does nothing.

Two ways out, both cheap:

  1. Keep 44 (what ships, and what WCAG asks). Remove .nav-btn and .ch-icon-btn from the 48px blocks at :561 and :567, and correct the :610 comment to say the group is 48 by house preference rather than by WCAG 2.5.5.
  2. Keep 48 (the house default for every other control in that block). Remove the min-width/min-height from the component rules at :890 and :1838. This does change the rendered size of the navbar buttons and the channel row icons, so it wants a look on mobile first.

Found while reviving tests/e2e/test-touch-targets.js, which had gone unrun (#2037). That suite demanded a blanket 48 and so failed on exactly these two. Rather than lower the blanket or hide the failures, it now has a MIN_OVERRIDES table pinning these two at their effective 44 with a pointer here, so a third selector dropping to 44 still fails the build. Whichever option is taken above, that table entry should be removed in the same change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: infraDocker, deployment, config, infrastructuretype: bugSomething is broken

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions