Skip to content

Add elevation to Link app bar #11042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add elevation to Link app bar #11042

wants to merge 1 commit into from

Conversation

tillh-stripe
Copy link
Collaborator

@tillh-stripe tillh-stripe commented Jun 30, 2025 •
edited
Loading

Summary

This pull request adds an elevation of the Link app bar based on the scroll state of the content.

Motivation

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screen recording

Screen_recording_20250630_211528.mp4

Changelog

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 30, 2025 •
edited
Loading

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   2.1 MiB │   2.1 MiB │  0 B │   4.3 MiB │   4.3 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 302.9 KiB │ 302.9 KiB │  0 B │   457 KiB │   457 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.7 KiB │   7.7 KiB │  0 B │   7.4 KiB │   7.4 KiB │  0 B 
    other │  95.8 KiB │  95.8 KiB │ -3 B │ 183.5 KiB │ 183.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.8 MiB │   9.8 MiB │ -3 B │  21.8 MiB │  21.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20682 │ 20682 │ 0 (+0 -0) 
   types │  6503 │  6503 │ 0 (+0 -0) 
 classes │  5266 │  5266 │ 0 (+0 -0) 
 methods │ 31511 │ 31511 │ 0 (+0 -0) 
  fields │ 18239 │ 18239 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3646 │ 3646 │  0
APK 
   compressed    │   uncompressed   │                        
──────────┬──────┼───────────┬──────┤                        
 size     │ diff │ size      │ diff │ path                   
──────────┼──────┼───────────┼──────┼────────────────────────
 29.2 KiB │ -2 B │  64.6 KiB │  0 B │ ∆ META-INF/CERT.SF     
 25.9 KiB │ -2 B │  64.5 KiB │  0 B │ ∆ META-INF/MANIFEST.MF 
  1.2 KiB │ +1 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA    
──────────┼──────┼───────────┼──────┼────────────────────────
 56.2 KiB │ -3 B │ 130.4 KiB │  0 B │ (total)

@tillh-stripe tillh-stripe force-pushed the tillh/link-app-bar-elevation branch from 8f97f76 to d2623b2 Compare June 30, 2025 19:18
@tillh-stripe tillh-stripe marked this pull request as ready for review June 30, 2025 19:18
@tillh-stripe tillh-stripe requested review from a team as code owners June 30, 2025 19:18
@tillh-stripe tillh-stripe force-pushed the tillh/link-app-bar-elevation branch from d2623b2 to 686c388 Compare June 30, 2025 19:35
@tillh-stripe tillh-stripe force-pushed the tillh/link-app-bar-elevation branch from 686c388 to 98b6674 Compare June 30, 2025 20:14
carlosmuvi-stripe
carlosmuvi-stripe reviewed Jun 30, 2025
View reviewed changes
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe left a comment

Choose a reason for hiding this comment

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

looking great! a couple nits.

paymentsheet/src/main/java/com/stripe/android/link/ui/LinkAppBar.kt
Comment on lines +160 to +162
Column(
modifier = Modifier.padding(bottom = 20.dp),
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is so that the elevation shadow is a bit more separated from the Link logo / toolbar content right?

paymentsheet/src/main/java/com/stripe/android/link/ui/LinkContentScrollHandler.kt

import androidx.compose.runtime.compositionLocalOf

internal val LocalLinkContentScrollHandler = compositionLocalOf<((Boolean) -> Unit)?> { null }
Copy link
Collaborator

Choose a reason for hiding this comment

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

The typing (nullable boolean lambda) confused me a little bit at the beginning - can we add a comment / create a type alias for the lambda?

...eenshotTest_testLinkAppBarState[LinkAppBarWithBackButtonAndTitle][DarkTheme,DefaultFont].png
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure why these changed colors 🤔

lng-stripe
lng-stripe reviewed Jun 30, 2025
View reviewed changes
paymentsheet/src/main/java/com/stripe/android/link/LinkActivityViewModel.kt
Comment on lines +124 to +126
fun onContentScrolled(canScrollUp: Boolean) {
_linkAppBarState.update {
it.copy(isElevated = canScrollUp)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was concerned this would be called on every scroll event and allocate a ton of objects, but I see that it's actually only invoked at the call site when canScrollUp is changed. Can we rename this to onContentCanScrollBackwardChanged() ("backward" instead of "up" to align with the library terminology)?

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

@lng-stripe lng-stripe lng-stripe left review comments

@carlosmuvi-stripe carlosmuvi-stripe carlosmuvi-stripe left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tillh-stripe @lng-stripe @carlosmuvi-stripe