Skip to content

fix(mobile): wrap long tooltip data and example pattern so the page doesn't widen - #991

Merged
ErikBjare merged 1 commit into
ActivityWatch:masterfrom
TimeToBuildBob:fix/event-tooltip-mobile-overflow
Sep 23, 2026
Merged

ErikBjare merged 1 commit into
ActivityWatch:masterfrom
TimeToBuildBob:fix/event-tooltip-mobile-overflow

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Partial fix for #984 (the layout part; see "Not covered" below).

Problem

On mobile, tapping a timeline event opens a tooltip whose data line (usually a long JSON string or URL) has no break opportunities. vis-timeline ships div.vis-tooltip { white-space: nowrap }, so the existing max-width: 400px on the tooltip never wrapped anything: the text overflowed the box and made the whole page wider than the viewport. The event editor modal then sizes itself to the widened page and ends up off screen.

The second symptom in the issue, the "Always count as active pattern" example expression on /#/settings/categorization stretching the page, comes from the example sitting in a text-nowrap span.

Change

  • VisTimeline.vue: tooltip gets white-space: normal, overflow-wrap: anywhere, box-sizing: border-box, and max-width: min(400px, calc(100vw - 20px)) so it can't exceed the viewport on phones.
  • ActivePatternSettings.vue: drop text-nowrap from the example span so it wraps.

Verification

Reproduced in headless Chrome with the real vis-timeline CSS and a tooltip holding a 180-char unbroken title plus a 120-char URL, in a 375px-wide frame:

document scroll width tooltip right edge
before 2379px 422px (text overflowing far beyond)
after 375px 365px

vue-cli-service lint is clean on both files and jest --selectProjects jsdom passes (18 suites, 99 tests). There's no unit coverage for this CSS; the check above is the evidence.

Not covered

The issue also reports that editing an event through the modal opened from the timeline doesn't refresh the timeline, unlike the "Edit" button path. That's a separate data-flow bug and isn't touched here, so #984 should stay open for it.

…oesn't widen

vis-timeline sets white-space: nowrap on .vis-tooltip, so the existing
max-width: 400px never wrapped anything: long unbroken event data (JSON,
URLs) overflowed the tooltip and made the whole page wider than the
viewport on mobile, which in turn pushed the event editor modal off screen.
Wrap the tooltip text, and cap its width to the viewport.

The 'Always count as active pattern' example expression on
/settings/categorization was wrapped in text-nowrap and had the same effect.

Measured in headless Chrome at a 375px viewport with a 180-char unbroken
title: document scroll width 2379px before, 375px after.

Git-Session-Id: a074
@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.62%. Comparing base (22cb53b) to head (7c7a250).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #991   +/-   ##
=======================================
  Coverage   57.62%   57.62%           
=======================================
  Files          51       51           
  Lines        3219     3219           
  Branches      751      751           
=======================================
  Hits         1855     1855           
  Misses       1348     1348           
  Partials       16       16           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with both changes narrowly addressing mobile overflow without introducing an established regression.

Summary

This PR addresses two sources of horizontal page overflow on narrow screens:

  • It allows vis-timeline tooltip content to wrap and constrains the tooltip to the viewport.
  • It removes forced nowrap behavior from the active-pattern example.
  • No actionable correctness, security, or repository-rule violations were identified.

Reviews (1) · Last reviewed commit: "fix(mobile): wrap long tooltip data and ..."

@TimeToBuildBob

TimeToBuildBob commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

This PR adjusts CSS in two Vue components to prevent long unbroken text from widening the page on mobile. In VisTimeline.vue, the .vis-tooltip rule gains max-width: min(400px, calc(100vw - 20px)), box-sizing: border-box, white-space: normal, and overflow-wrap: anywhere. In ActivePatternSettings.vue, the example expression span loses its text-nowrap class so it can wrap.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No thread-worthy findings. Advisory notes follow; they are retained without opening review threads.

2 advisory findings (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — src/views/settings/ActivePatternSettings.vue

This is a fix(...) PR but no test files are included in the diff. Erik's feedback: 'where is the repro & fixes they are supposed to catch' (gptme#3441), 'that measurement should come with a regression test' (gptme#3446). Add a test that would have caught this bug. (Advisory: Erik merged all such PRs but consistently requested tests.)

Add a test file that reproduces the bug before the fix and passes after it.

How this was verified: static preflight: fix-commit + touched-files scan (rule 7)

⚠️ P2 medium — src/visualizations/VisTimeline.vue:29

The new max-width: min(400px, calc(100vw - 20px)) uses 100vw, which on mobile browsers includes the vertical scrollbar width (or the layout viewport width) and can be larger than the actual visible viewport width when a scrollbar is present. On desktop, 100vw includes the scrollbar width, so the tooltip can be up to ~15px wider than the visible area, potentially causing a horizontal scrollbar or slight overflow. On mobile, 100vw typically equals the layout viewport width, but the 20px margin may not account for all browser UI overlays. The observable consequence is that the tooltip may still slightly exceed the visible viewport on some devices, though the PR's main goal of preventing page widening is mostly achieved. This is a minor edge case; the fix is to use 100dvw or a percentage-based width, but the current approach is a reasonable heuristic.

max-width: min(400px, calc(100dvw - 20px));

How this was verified: Checked the CSS rule in the file and the PR description's verification table. The use of 100vw is a known issue on mobile where it includes scrollbar width. The PR's own verification used a 375px-wide frame without a scrollbar, so it wouldn't catch this.

Files changed (2) — the diff as I read it
  • src/views/settings/ActivePatternSettings.vue — Removes the text-nowrap class from the example expression span so it can wrap on narrow screens.
  • src/visualizations/VisTimeline.vue — Updates the .vis-tooltip CSS to cap width to viewport, allow wrapping, and use overflow-wrap: anywhere.

Reviewed 7c7a250381e4 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 22s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@ErikBjare
ErikBjare merged commit 23c95ed into ActivityWatch:master Sep 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants