Skip to content

fix(superuser): improve banner ux#115168

Merged
priscilawebdev merged 5 commits into
masterfrom
priscila/fix/de-1231-superuser-banner-issues
May 12, 2026
Merged

fix(superuser): improve banner ux#115168
priscilawebdev merged 5 commits into
masterfrom
priscila/fix/de-1231-superuser-banner-issues

Conversation

@priscilawebdev
Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev commented May 8, 2026

  • The strip now spans the full viewport width.
  • Updated the marquee copy from "You are in superuser mode" to "You
    are in superuser mode · hover for exit option" so users can discover
    how to exit.
  • Added cursor: help to help with discoverability of the exit option
  • The tooltip's arrow now points up at the strip instead of down,
    matching the popup's new position below the banner.

Before

Screen.Recording.2026-05-08.at.13.50.01.mov

After

Screen.Recording.2026-05-08.at.13.47.02.mov

closes DE-1231

Address visual and behavioral issues with the superuser marquee banner:

- Marquee text now adapts to viewport width using ResizeObserver
  instead of hardcoded 8 repetitions, removing the white gap on wide
  screens.
- Tooltip placement changed to bottom-start with containerDisplayMode
  block so the popup renders below the banner with a correct arrow
  direction and the strip fills the full width.
- Added cursor: help on the strip and 'hover to exit' hint in the
  marquee copy to signal the hover-for-info affordance.
- Pinned the strip to the top of its frame via align-items: flex-start
  so the tooltip anchors at the strip rather than the viewport bottom.

Refs DE-1231
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 8, 2026

DE-1231

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 8, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.47%

Change 'hover to exit' to 'hover for exit option' since hovering reveals
a tooltip containing an Exit button rather than exiting directly.

Refs DE-1231
@priscilawebdev priscilawebdev requested a review from natemoo-re May 8, 2026 12:06
@priscilawebdev priscilawebdev marked this pull request as ready for review May 8, 2026 12:06
@priscilawebdev priscilawebdev requested a review from a team as a code owner May 8, 2026 12:06
Copy link
Copy Markdown
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for tackling this!

Would be nice to refactor to useResizeObserver, but up to you.

Comment on lines +99 to +100
const observer = new ResizeObserver(update);
observer.observe(strip);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's refactor to use the lib?

import {useResizeObserver} from '@react-aria/utils';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh yes sure! thanks for the reminder 🙏

Comment thread static/gsApp/components/superuser/superuserWarning.tsx Outdated
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Comment thread static/gsApp/components/superuser/superuserWarning.tsx
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c95b106. Configure here.

'https://www.notion.so/sentry/Sentry-Rules-for-Handling-Customer-Data-9612532c37e14eeb943a6a584abbac99';

const SUPERUSER_MESSAGE = 'You are in superuser mode';
const SUPERUSER_MESSAGE = 'You are in superuser mode / Hover for more information';
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.

Hover instruction leaks into non-marquee alert context

Medium Severity

The updated SUPERUSER_MESSAGE ("…Hover for more information") is shared across two contexts. On line 110, when hasPageFrame is false, this message is injected into the AlertStore alert — which already displays WARNING_MESSAGE inline right next to it. In that path there's no hoverable marquee strip, so telling users to "hover for more information" is misleading — the information is already visible in the alert itself. The old message ("You are in superuser mode") worked correctly in both contexts.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c95b106. Configure here.

@priscilawebdev priscilawebdev force-pushed the priscila/fix/de-1231-superuser-banner-issues branch from 3a50b96 to 11b9b8f Compare May 12, 2026 06:24
@priscilawebdev priscilawebdev enabled auto-merge (squash) May 12, 2026 06:25
Comment on lines +93 to +96
setMarqueeCount(currentCount => {
const repWidth = text.offsetWidth / currentCount;
return repWidth ? Math.ceil(stripWidth / repWidth) + 2 : currentCount;
});
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.

Bug: Reading text.offsetWidth inside the setMarqueeCount functional updater can cause a race condition during rapid resizes, as the DOM may not reflect the pending state.
Severity: LOW

Suggested Fix

Capture the text.offsetWidth value before calling setMarqueeCount. Pass this measurement into the state updater or use it to calculate the new count outside the updater. This ensures that the DOM measurement corresponds to the state that was rendered when the measurement was taken, avoiding reliance on currentCount inside the updater for this calculation.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/gsApp/components/superuser/superuserWarning.tsx#L93-L96

Potential issue: The `setMarqueeCount` functional updater reads `text.offsetWidth`
directly from the DOM. During rapid viewport resizing, multiple `ResizeObserver`
callbacks can queue state updates before React commits any DOM changes. This creates a
race condition where the updater's `currentCount` argument reflects a pending state, but
`text.offsetWidth` reflects the older, committed DOM state. As a result, the calculation
`repWidth = text.offsetWidth / currentCount` is incorrect, leading to a miscalculation
of the required repetitions and causing visible gaps or unnecessary DOM nodes in the
marquee.

@priscilawebdev priscilawebdev merged commit 546d017 into master May 12, 2026
72 checks passed
@priscilawebdev priscilawebdev deleted the priscila/fix/de-1231-superuser-banner-issues branch May 12, 2026 06:32
nikkikapadia pushed a commit that referenced this pull request May 12, 2026
- The strip now spans the full viewport width.
- Updated the marquee copy from "You are in superuser mode" to "You are in superuser mode / Hover to exit or learn more"
- Added cursor: help to help with discoverability of the exit option
- The tooltip's arrow now points up at the strip instead of down,
matching the popup's new position below the banner.

**Before**




https://github.com/user-attachments/assets/f65f44ee-4563-4cf9-affb-3355b977963a



**After**


https://github.com/user-attachments/assets/16d746a9-d7e0-43bf-9dd9-077da3cf03b6



closes DE-1231

---------

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants