Skip to content

Conversation

@CDRussell
Copy link
Member

@CDRussell CDRussell commented Dec 10, 2024

Task/Issue URL: https://app.asana.com/0/488551667048375/1208758825735780/f

Description

This PR tweaks the layout params for the password recycler view so that the whole view content, already encapsulated in a NestedScrollView, is scrollable.

  • Before, you had to scroll on the list view directly, and then only the passwords list would scroll while the rest of the views above were fixed.
  • Now, the entire view (including the toggle) is scrollable

Steps to test this PR

  • Visit password management view when you have 0 passwords; verify it looks fine and unchanged from before
  • Add a single password; verify the password appears immediately below the horizontal divider
  • Add enough passwords that they appear down below the screen.
  • Verify that scrolling scrolls the whole content, and not just the password list itself
  • Verify you can scroll on the list, or scroll on the sync panel, or the text etc… and the whole page scrolls the same way

Screenshots

Current behavior

current

### New behavior
after

Copy link
Member Author

CDRussell commented Dec 10, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CDRussell CDRussell marked this pull request as ready for review December 10, 2024 12:59
Copy link
Contributor

@cmonfortep cmonfortep left a comment

Choose a reason for hiding this comment

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

LGTM

@CDRussell CDRussell merged commit aa6cf2c into develop Dec 16, 2024
8 checks passed
@CDRussell CDRussell deleted the feature/craig/make_all_password_management_screen_scrollable branch December 16, 2024 15:40
CDRussell added a commit that referenced this pull request Jan 8, 2025
…en" (#5445)

Task/Issue URL:
https://app.asana.com/0/608920331025315/1209106756565265/f

### Description
Reverts changes made in #5377
as it introduces performance issues on the password management screen
with larger password list sizes. Depending on the device / list size, it
can be crippling performance.

### Steps to test this PR
- [x] install `internal` variant to make the next step easier
- [x] Add 500+ passwords (can be done in `Autofill Dev Settings` screen,
using the **Add a lot of sample logins** button)
- [x] Visit password management screen and verify there isn’t a huge
delay in rendering the password list

For contrast, if you try this on `develop` you will notice a large perf
issue.

Co-authored-by: Craig Russell <1336281+CDRussell@users.noreply.github.com>
karlenDimla pushed a commit that referenced this pull request Jan 8, 2025
…en" (#5445)

Task/Issue URL:
https://app.asana.com/0/608920331025315/1209106756565265/f

### Description
Reverts changes made in #5377
as it introduces performance issues on the password management screen
with larger password list sizes. Depending on the device / list size, it
can be crippling performance.

### Steps to test this PR
- [x] install `internal` variant to make the next step easier
- [x] Add 500+ passwords (can be done in `Autofill Dev Settings` screen,
using the **Add a lot of sample logins** button)
- [x] Visit password management screen and verify there isn’t a huge
delay in rendering the password list

For contrast, if you try this on `develop` you will notice a large perf
issue.

Co-authored-by: Craig Russell <1336281+CDRussell@users.noreply.github.com>
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