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

[ENG-3384] fix: UI issues found in 0.24 #698

Merged
merged 17 commits into from
Dec 5, 2023

Conversation

teebszet
Copy link
Member

@teebszet teebszet commented Dec 4, 2023

πŸ”˜ PR Type

  • Bugfix

πŸ“œ Background

Issue Link: ENG-3384

πŸ”„ Changes

  • fix: sticky buttons and padding on confirm btc screen, confirm send rare sats screen, confirm send ordinal screen
  • fix: always use responsive container for confirm btc screen because ledger accounts require it, and fix double scroll bar, hide back button issues when on a full screen tab
  • [ @abdulhaseeb4239 ] fix: hide the sat bundle on review for a btc send screen

Impact:

  • responsive send layout is shared between: send ordinal, send rare sats, send nft, confirm btc send, confirm rare sats send, confirm ordinal send
  • the responsive layouts cover:
    • extension popup screen (docked to extension icon)
    • extension full screen tabs (incl. ledger for send/confirm screens)
    • extension popup windows (floating and height is responsive, but currently only the send btc popup will be responsive to screen width too)
  • also had to refactor the global CSS and containers. I checked all the main screens incl. login, settings, home, stacking, collectibles, and restore wallet flow
  • also fixed some cases where back buttons / account headers / bottom nav bars were not supposed to be showing, or were supposed to be disabled

πŸ–Ό Screenshot / πŸ“Ή Video

send btc fixed:

Screen.Recording.2023-12-04.at.3.21.44.PM.mov

send rare sats:
image

confirm rare sats:
image

popup windows:

Screen.Recording.2023-12-04.at.11.10.29.PM.mov

βœ… Review checklist

Please ensure the following are true before merging:

  • Code Style is consistent with the project guidelines.
  • Code is readable and well-commented.
  • No unnecessary or debugging code has been added.
  • Security considerations have been taken into account.
  • The change has been manually tested and works as expected.
  • Breaking changes and their impacts have been considered and documented.
  • Code does not introduce new technical debt or issues.

@abdulhaseeb4239
Copy link
Contributor

abdulhaseeb4239 commented Dec 4, 2023

There seems to be a UI issue in rare sat confirm screen's buttons

Screenshot 2023-12-04 at 2 24 28 PM

@abdulhaseeb4239
Copy link
Contributor

The bottom tabs are hidden for some reason.

Screenshot 2023-12-04 at 2 39 55 PM

Comment on lines -414 to -421
{
path: 'confirm-ordinal-tx/:id',
element: (
<AuthGuard>
<ConfirmOrdinalTransaction />
</AuthGuard>
),
},
Copy link
Member Author

Choose a reason for hiding this comment

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

code should always use /nft-dashboard/confirm-ordinal-tx/:id route

@teebszet teebszet merged commit 91deed7 into release/v0.25.0 Dec 5, 2023
@teebszet teebszet deleted the tim/eng-3384-issues-found-in-024 branch December 5, 2023 03:38
@teebszet teebszet changed the title Tim/eng 3384 issues found in 024 [ENG-3384] fix: UI issues found in 0.24 Dec 5, 2023
This was referenced Dec 6, 2023
@github-actions github-actions bot mentioned this pull request Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants