Skip to content

fix: pinned tabs with viewpager and drop shadow#968

Open
jvsena42 wants to merge 12 commits into
masterfrom
fix/design-review-181
Open

fix: pinned tabs with viewpager and drop shadow#968
jvsena42 wants to merge 12 commits into
masterfrom
fix/design-review-181

Conversation

@jvsena42
Copy link
Copy Markdown
Member

@jvsena42 jvsena42 commented May 25, 2026

Addresses #916

This PR:

  1. Pins the tab rows on All Activity, Shop and Settings so content scrolls behind a frosted (haze) header with a drop shadow
  2. Adds swipe-between-tabs (HorizontalPager) on the Shop screen, matching Settings
  3. Adds the missing icon to the Add Widget button
  4. Adds 42dp bottom spacing to the Shop list

Description

Design review follow-ups for Android v2.2.0, generalized to every screen with a tab row.

  • A shared PinnedTabsScaffold keeps the tab row (and the Activity search field) pinned while content scrolls behind it. The header uses an iOS-style haze blur tinted to the black background — so it only frosts when content is actually behind it — plus a gradient drop shadow below the tabs.
  • Shop switches tabs via a HorizontalPager; on the Map tab the pager swipe is disabled so the map can be panned. All Activity keeps its existing swipe-to-change-tab gesture.
  • The Add Widget button shows a plus icon, and the Shop list gets 42dp of bottom padding.

Preview

activities.webm
home.webm
shop.webm

QA Notes

Manual Tests

  • 1. All Activity → swipe / tap across tabs and scroll: content scrolls behind the frosted, pinned tabs.
  • 2. regression: All Activity → search: list filtering still animates.
  • 3a. Shop → swipe / tap between Shop and Map tabs.
    • 3b. Shop → Map tab → pan the map: no pager conflict; tap Shop tab to return.
  • 4. Shop → scroll to bottom: last item has clear bottom spacing.
  • 5. Home → Add Widget button shows a plus icon left of the label.

Automated Checks

  • UI-only; no automated coverage changed. Ran compileDevDebugKotlin, detekt, testDevDebugUnitTest locally — all pass.

@jvsena42 jvsena42 self-assigned this May 25, 2026
@jvsena42 jvsena42 marked this pull request as ready for review May 25, 2026 17:53
@jvsena42 jvsena42 added this to the 2.4.0 milestone May 25, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c89dd8837c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

onEmptyActivityRowClick = onEmptyActivityRowClick,
contentPadding = PaddingValues(top = 0.dp),
listState = listState,
contentPadding = PaddingValues(top = topPadding + 16.dp),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Offset empty activity state below pinned header

After moving All Activity into PinnedTabsScaffold, the screen relies on contentPadding to keep content below the pinned filter/tabs, but that padding is only honored by the list branch in ActivityListGrouped. When filters produce no results, the fallback empty text is rendered at the top of the scaffold and is overlapped by the pinned header, so users can’t clearly see the "no activity" state. This is reproducible by opening All Activity and applying filters that return zero rows.

Useful? React with 👍 / 👎.

)
PinnedTabsScaffold(
header = {
Column(modifier = modifier.fillMaxWidth()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid reusing outer modifier in header container

ShopDiscoverScreen now applies the same modifier parameter to both ScreenColumn and the pinned header Column. This duplicates any caller-provided layout/semantics modifiers across two nodes; for example, a size modifier like fillMaxSize can make the header measure as full height and push pager content off-screen, while semantics/test tags become duplicated. The header should use a local Modifier.fillMaxWidth() instead of reusing the screen-level modifier.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

added 2 nits for early review; there's also some relevant AI bot comments.

.fillMaxWidth()
.zIndex(2f)
.hazeEffect(state = hazeState, style = hazeStyle)
.onSizeChanged { headerHeight = with(density) { it.height.toDp() } }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: should use subcompose layout to avoid re-renders during measurements composition pass.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: there's also a .scaffold package for this kind of things.

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