Skip to content

fix(history): long token names widen the popup; asset detail hidden behind transaction sheet - #3035

Merged
CassioMG merged 3 commits into
masterfrom
fix/history-popup-width
Oct 12, 2026
Merged

CassioMG merged 3 commits into
masterfrom
fix/history-popup-width

Conversation

@CassioMG

@CassioMG CassioMG commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR

Two transaction history display fixes.

  1. A long token name made the popup wider. If an account's history included a token with a long symbol, such as a Soroswap LP token (native-USDC-SOROSWAP-LP), opening History made the popup about 500px wide instead of 360px. Chrome sizes the popup to the narrowest width its content fits in. The amount on each history row stays on one line, and the 45% width cap on it doesn't count when Chrome measures the content, so the full symbol set the popup's width. Each row now takes its width from the page instead (CSS contain: inline-size), so the amount is cut off with an ellipsis, as it already was in the sidebar and fullscreen. A symbol with no hyphens or spaces now wraps inside the label instead of running off the row and hiding the amount.
  2. Transaction details opened from an asset's page had a solid black background. Opening a transaction from Asset Details replaced the whole page with the details panel, so the see-through background behind the panel had nothing to show. Asset Details now stays on screen behind the panel, as it does on the History screen, and the background also dims the Send/Swap buttons.

Before:

history-bugs-720p.mov

After:

history-bugs-fixed.mov
Implementation details (for agents)

What changed

  • History row width. Adds contain: inline-size to .HistoryItem: HistoryItem/styles.scss#L5-L8
    • The amount column is white-space: nowrap with max-width: 45% (#L44-L45). When Chrome measures the content's minimum width, a percentage max-width has nothing to resolve against and is ignored. So the row's minimum width was the full amount string.
    • Chromium sizes extension popups from the document's min-content width. With AutoSizeUsesScrollWidthForOverflow off, it uses LayoutView::ComputeMinimumWidth(). With it on, it expands the initial containing block to min-content while auto-sizing.
    • Measured with one native-USDC-SOROSWAP-LP row: the document's min-content width was 500.5px. Hiding the amount brought it to 360px. Adding min-width: 0 to the flex rows changed nothing. With containment the row's own min-content width no longer depends on its text, and at a fixed width the layout is identical.
  • Symbols with no break opportunity. Adds min-width: 0 to .HistoryItem__row and .HistoryItem__description, and overflow-wrap: anywhere to the description: HistoryItem/styles.scss#L10-L30
    • Containment keeps the popup at 360px, but the flex items inside the row still defaulted to min-width: auto. A symbol like SUPERLONGMEMECOINNAMEFORTESTING couldn't shrink below its own width, so the label ran 57px past the row and the amount column (whose minimum is 0 because of overflow: hidden) was squeezed to 0px.
    • With the change, the label wraps and the amount keeps its 140px. A hyphenated symbol still wraps at its hyphens, as before.
  • Asset Details sheet. AssetDetail/index.tsx
    • Before, the component returned only the SlideupModal whenever activeAssetId was set. That unmounted the view inside the account Sheet and left only its opaque gray-01 background.
    • The transaction SlideupModal is now always mounted inside the View, next to the existing balance-info sheet. This matches how AccountHistory renders it.
    • It passes hasBackdrop, so the backdrop is the fixed full-screen variant (z-index: 29). Otherwise the sticky Send/Swap bar (z-index: 10) would sit above the default backdrop (z-index: 1) while the sheet animates in and out.
    • Closing the sheet now returns to the same Asset Details instance instead of remounting it.

Verification

  • New e2e test, Long token codes fit in the popup, with a hyphenated symbol and one with no break opportunity:
    • The popup width check fails before the containment fix (500.5px with the hyphenated row alone) and passes after (360px).
    • The per-row check (no overflow, amount wider than 0) fails with containment alone (the unbroken row overflows by 57px) and passes with the min-width/overflow-wrap change.
    • It measures the document's min-content width rather than the popup window. Headless Chromium doesn't re-auto-size an action popup on in-app navigation, so the window itself can't be asserted on there.
  • New unit test, keeps the asset detail behind an opened transaction: fails before the fix because AssetDetail is unmounted, passes after.
  • Built extension at 360×600:
    • Asset Details is dimmed behind the transaction sheet, matching History.
    • elementFromPoint over Send and Swap hits the sheet.
    • The active backdrop is fixed with z-index: 29.
    • Tapping the backdrop closes the sheet back to Asset Details.
  • The reported account in Chrome 154 no longer widens the popup.
  • The home token list with a long-symbol token was checked manually and doesn't widen the popup.
  • Test runs:
    • yarn test:ci: 1945 passed.
    • e2e accountHistory, sendPayment, reviewTxFees, loadAccount: all pass. The only skips are the existing test.skip/test.fixme.
    • Existing History screenshot baselines are unchanged.
  • Not tested in Firefox.

🤖 Generated with Claude Code

CassioMG and others added 2 commits October 8, 2026 11:01
Chrome sizes the extension popup from the document's min-content width.
A history row's amount is nowrap, and its percentage max-width doesn't
cap its min-content contribution, so a long token symbol such as
"native-USDC-SOROSWAP-LP" pushed the history page to ~500px. Size each
row from its container with `contain: inline-size`; layout at a fixed
width (sidebar, fullscreen) is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on sheet

Opening a transaction from an asset's detail returned only the slide-up
sheet, unmounting the detail view, so the translucent backdrop showed an
empty background. Keep the view mounted and render the sheet alongside
it, as the History screen does. The sheet uses the full-screen backdrop
so it also dims the sticky Send/Swap bar.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 8, 2026 19:59
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-3cdc64e3708b508c8b32
Backend: V1 prod + V2 beta (no sandbox configured for @CassioMG). SDF collaborators only — install instructions in the release description.

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.

🔵 Needs a closer look

Underlying Asset Details controls remain keyboard-focusable while the transaction sheet is open.

0 open findings

What changed in this PR

Fixes long-token popup widening and preserves Asset Details behind transaction sheets.

Changes:

  • Adds inline-size containment to history rows.
  • Keeps Asset Details mounted with a full-screen backdrop.
  • Adds unit and E2E regression tests.
File Description
extension/​src/​popup/​components/​accountHistory/​HistoryItem/​styles.scss Prevents long token names from widening the popup.
extension/​src/​popup/​components/​account/​AssetDetail/​index.tsx Preserves the asset view behind transaction details.
extension/​src/​popup/​components/​__tests__/​AssetDetail.test.tsx Tests preserved asset-detail rendering.
extension/​e2e-tests/​accountHistory.test.ts Tests long-token popup sizing.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@CassioMG CassioMG self-assigned this Oct 8, 2026
@CassioMG
CassioMG requested a review from a team October 8, 2026 20:09
@CassioMG

CassioMG commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

// Chrome sizes the popup to its content's min-content width, and the
// nowrap amount's percentage max-width doesn't cap that, so a long token
// code would widen the whole popup. Size the row from its container only.
contain: inline-size;

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.

Long token symbols without hyphens still overflow the row

TL;DR: The popup now keeps its width, but the row's content can't shrink to fit inside it. A token symbol with no spaces or hyphens runs off the right edge, and the amount column is squeezed to zero width. SEP-41 puts no length limit on symbol(), so any custom Soroban token can do this. The new e2e test doesn't catch it because its symbol is hyphenated, so it wraps at the hyphens. The test still passes with a 31-character unbroken symbol.

Steps to reproduce:

  1. In the new e2e test "A long token code does not widen the popup", change LONG_CODE to "SUPERLONGMEMECOINNAMEFORTESTING".
  2. Run it. It still passes.
  3. Look at the History screen: the label runs off the screen and the amount isn't visible.

Detailed explanation (for agents)

Root cause: contain: inline-size fixes the popup width, but the flex items inside the row still use the default min-width: auto, so they can't shrink below their content.

<div className="HistoryItem__row">
<div className="HistoryItem__row HistoryItem--space-between">
<div className="HistoryItem__row">
<div className="HistoryItem__icon">{operation.rowIcon}</div>
<Text
as="div"
size="md"
weight="regular"
addlClassName="HistoryItem__description"
>
<span data-testid="history-item-label">{operation.rowText}</span>
<Text
as="div"
size="xs"
weight="regular"
addlClassName="HistoryItem--action"
>
<ActionIcon actionType={operation.actionIcon} />
{operation.action}
</Text>
</Text>
</div>
<div

  • The icon+label .HistoryItem__row and .HistoryItem__description both have min-width: auto. With an unbreakable label, that minimum is the full label width.
  • .HistoryItem__amount has overflow: hidden, so its automatic minimum is 0. When there isn't enough space, it is the column that shrinks, all the way to nothing.

The label comes straight from the token contract: rowText: code where code = isNative ? "XLM" : symbol.

Measured in the PR's e2e test at ae141d16f, 360×600 viewport, with a measurement added after the existing width check:

LONG_CODE document min-content width row scrollWidth / clientWidth amount width label right edge / row right edge
native-USDC-SOROSWAP-LP 360 312 / 312 140px 185 / 336
SUPERLONGMEMECOINNAMEFORTESTING 360 369 / 312 0px 393 / 336

Both runs pass expect(popupWidth).toBe(360).

Suggested fix (checked with the same measurement):

&__row {
  width: 100%;
  min-width: 0;
  ...
}

&__description {
  flex: 1;
  min-width: 0;
  overflow-wrap: anywhere;
  ...
}

With this change the unbroken-symbol case measures row scrollWidth 312, amount width 140px, and label right edge 190 (wrapped). The hyphenated case is unchanged.

Test: in the e2e test, use an unbroken LONG_CODE, or add a second case with one. Also assert that history-item-amount-component has a non-zero width. The current assertion only checks the popup width, so it can't catch the amount being squeezed to zero.

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.

Pretty minor but confirmed this could potentially be an issue:
Screenshot 2026-10-09 at 7 04 34 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7719168 with your suggestion: min-width: 0 on .HistoryItem__row and .HistoryItem__description, plus overflow-wrap: anywhere on the description.

The e2e test now stubs two rows, native-USDC-SOROSWAP-LP and SUPERLONGMEMECOINNAMEFORTESTING. Besides the popup width, it checks each row for no overflow and an amount wider than 0. It fails with containment alone (the unbroken row overflows by 57px) and passes with the CSS change. My measurements match yours: the row stays at 312px, the amount at 140px, and the label wraps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2026-10-09 at 17 57 57 Screenshot 2026-10-09 at 17 57 38 Screenshot 2026-10-09 at 17 55 31

{/* The full-screen backdrop also covers the sticky Send/Swap bar */}
<SlideupModal
isModalOpen={activeOperation !== null}
setIsModalOpen={() => setActiveAssetId(null)}

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.

Tapping another transaction while the sheet is closing does nothing

Low severity: the tap is silently lost, but tapping the same row again opens it.

TL;DR: Now that the transaction list stays visible behind the sheet, a user can close one transaction and tap another within a fraction of a second. If the second tap lands during the sheet's 200ms closing animation, it is swallowed and no sheet opens. Before this PR this couldn't happen, because the list wasn't on screen while a transaction was selected.

Steps to reproduce:

  1. Open an asset's detail page that has at least two history rows.
  2. Tap row A to open its transaction sheet.
  3. Tap the dimmed backdrop to close it, then tap row B within about 200ms.
  4. No sheet opens for B. A second tap on B opens it normally.

Detailed explanation (for agents)

Root cause: the backdrop click closes the sheet in two steps. It sets the modal's internal isOpen to false right away, and only clears activeAssetId 200ms later in a setTimeout:

<LoadingBackground
onClick={() => {
setIsOpen(false);
setTimeout(() => {
setIsModalOpen(false);
}, SLIDEUP_MODAL_TRANSITION_MS);
}}
isActive={isOpen}
isFullScreen={hasBackdrop}
/>

<SlideupModal
isModalOpen={activeOperation !== null}
setIsModalOpen={() => setActiveAssetId(null)}
hasBackdrop
>
<TransactionDetail
activeOperation={activeOperation}
networkDetails={networkDetails}
/>
</SlideupModal>

A tap on row B inside that window then plays out like this:

  1. activeAssetId goes from A to B. isModalOpen (activeOperation !== null) stays true.
  2. Because isModalOpen didn't change, the useEffect that copies it into isOpen doesn't run, and the sheet stays closed. (L29-L31)
  3. The pending timeout from closing A fires and sets activeAssetId to null, which drops B.

Before this PR, AssetDetail returned only the sheet while activeAssetId was set, so no row was there to tap during the close:

return activeAssetId ? (
<SlideupModal
isModalOpen={activeOperation !== null}
setIsModalOpen={() => setActiveAssetId(null)}
>
<TransactionDetail
activeOperation={activeOperation}
networkDetails={networkDetails}
/>
</SlideupModal>
) : (

Deterministic repro: a throwaway Playwright spec using the data from "Orders failed transactions by date alongside successful ones", run at ae141d16f with real mouse clicks:

  1. Click row 0 and wait for .SlideupModal to have the open class.
  2. page.mouse.click on the backdrop, wait N ms, then page.mouse.click on the center of row 1.
  3. Wait 1s and read the sheet's class.
gap between backdrop click and row click sheet class 1s later
400ms SlideupModal open
50ms SlideupModal closed
(then a second tap on row 1) SlideupModal open

Fix direction (not tested): selecting a row during the close needs to both cancel the pending clear of activeAssetId and open the sheet again. Today the delayed close callback clears whatever id is active by then. Making it clear only the id it was closing would stop it dropping B, but on its own the sheet still wouldn't reopen, because isModalOpen never went false.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not reachable with a real tap, so I'd leave this out of this PR. The race is real in the code. A programmatic .click() on row B 50ms after closing reproduces it exactly as you describe. But a pointer tap can't land in that window. The full-screen backdrop stays on top of the rows for about 370ms after the backdrop tap, and the pending clear fires at 200ms. So in the 50ms repro, the second tap goes to the fading backdrop and the row's click handler never runs.

Measurements

Built extension at ae141d1, 360×600, three USDC rows in Asset Details. I recorded the click target with a capture-phase listener on document. Each run: open row A, tap the backdrop with a real mouse click, wait the given gap, then tap row B's center.

gap after backdrop tap click target sheet 1s later
50ms LoadingBackground closed
200ms LoadingBackground closed
300ms LoadingBackground closed
350ms LoadingBackground closed
400ms history-item open
450ms history-item open
600ms history-item open

.LoadingBackground has transition: all (0.3s ease-out after a 0.1s delay), so its z-index animates from 29 down to -1 instead of dropping at once. Sampled under row B's center: 29 until about 110ms, 0 at about 370ms, and -1 from about 395ms, which is when the row becomes the hit target. History rows have no tabIndex or role, so there's no keyboard path either. The History screen uses the same SlideupModal close sequence.

If we want this not to depend on the transition timing, the fix belongs in SlideupModal, since History shares it. It would keep the pending close timeout in a ref and cancel it when the modal is reopened. That fits better as a follow-up than in this PR.

A symbol like "SUPERLONGMEMECOINNAMEFORTESTING" couldn't shrink below
its own width, so it ran past the row and squeezed the amount to zero
width. Let the label column shrink and break the symbol anywhere. The
e2e test now covers an unbroken symbol and checks each row for
overflow and a visible amount.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 10, 2026 01:00

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.

🔵 Needs a closer look

Strengthen the asset detail test to verify the modal opens and the active backdrop is present.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Medium severity Assert transaction modal opens and displays an active backdrop

extension/​src/​popup/​components/​__tests__/​AssetDetail.test.tsx:280

This only proves that the transaction detail is mounted in the DOM; SlideupModal always renders its children even when closed, so it would also pass if the click failed to open the sheet. Since this change adds both hasBackdrop and the layering needed to cover the sticky action bar, assert that the transaction modal has the open class and that an active full-screen backdrop is present (and ideally that the backdrop closes back to AssetDetail).

🧠 Review effort: Lite

@CassioMG
CassioMG merged commit f7bbf13 into master Oct 12, 2026
12 checks passed
@CassioMG
CassioMG deleted the fix/history-popup-width branch October 12, 2026 16:35
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.

3 participants