Skip to content
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.

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

fix: push the menu up if there isn't enough space at the bottom #22889

Merged
merged 1 commit into from Feb 16, 2024

Conversation

montelaidev
Copy link
Contributor

@montelaidev montelaidev commented Feb 9, 2024

Description

This pr fixes the issue where the remove account menu option is cut off if there isn't enough screen space.

Related issues

Fixes: 22836

Manual testing steps

Pop-up view:

  1. Create 3 HD accounts for a total of 4
  2. Add 1 imported account
  3. Click on the account menu
  4. Click on 3 dots next to imported account
  5. See that remove button is visible
  6. Click to remove and then confirm
  7. Ensure removed account is no longer in the accounts drop-down list

Full screen view:

  1. Create 3 HD accounts for a total of 4
  2. Add 1 imported account
  3. From 3 dots menu open extension in expanded view
  4. Click on the accounts menu
  5. Click on 3 dots next to imported account
  6. See that remove button is visible
  7. Click to remove and then confirm
  8. Ensure removed account is no longer in the accounts drop-down list

Hidden Account in pop-up view:

  1. Create 3 HD accounts for a total of 4
  2. Add 1 imported account
  3. Click on the accounts menu
  4. Click on 3 dots next to imported account
  5. Select Hide account
  6. From accounts menu, expand Hidden accounts
  7. Click on 3 dots next to the hidden imported account
  8. Make sure that remove is still presented and available
  9. Click to remove and then confirm
  10. Ensure removed account is no longer in the accounts drop-down list

Screenshots/Recordings

Before

Screenshot 2024-02-09 at 22 04 51

After

Screenshot 2024-02-09 at 22 04 12

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

github-actions bot commented Feb 9, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

Builds ready [6d5d37d]
Page Load Metrics (838 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint84144112189
domContentLoaded115522115
load73310918388239
domInteractive115522116
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 8 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@darkwing darkwing added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Feb 9, 2024
@montelaidev montelaidev marked this pull request as ready for review February 9, 2024 15:19
@montelaidev montelaidev requested a review from a team as a code owner February 9, 2024 15:19
Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

Wonderful! Looks and functions great!

@darkwing darkwing merged commit be6691f into develop Feb 16, 2024
68 of 71 checks passed
@darkwing darkwing deleted the fix/22836 branch February 16, 2024 22:42
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2024
@metamaskbot metamaskbot added the release-11.12.0 Issue or pull request that will be included in release 11.12.0 label Feb 16, 2024
@plasmacorral
Copy link
Contributor

Updated manual testing scenarios to cover pop-up view, expanded view, and removing a hidden account

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-11.12.0 Issue or pull request that will be included in release 11.12.0 team-accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Unable to see remove account option for last account in list
6 participants