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

647 Refactor pull-up sheets to use dynamic height #688

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jeden
Copy link
Contributor

@jeden jeden commented Dec 19, 2024

Related Issue

#647

Summary of Changes

Refactored the implementation of the half-sheet so that its height was dynamically set depending on the content.
This solution doesn't work with views having scroll views (even when combined with a geometry reader) because of the intrinsic size. These views will use the existing PresentHostingController with a fixed-sized sheet that can expand from half to full height.

Need Regression Testing

  • Yes
  • No

All views (listed below) should be checked to verify the correct layout.

Risk Assessment

  • Low
  • Medium
  • High

Additional Notes

Views migrated to use the dynamic pull-up sheet (in RouteMap unless otherwise specified):

  • MoveAssetsView in BrowserViewController
  • SyncAddDeviceView
  • BuyProvderView
  • StakeAmountView.StakeSetupView
  • JailbreakAlertView
  • MoveAssetsView
  • MoveTokenView
  • NFTTransferView
  • ChildAccountLinkView
  • NetworkSwitchPopView

Views automatically using the dynamic pull-up sheet (without any refactoring on a per-view basis):

  • AccountKeysView showing AccountKeyRevokeView
  • AddTokenView showing AddTokenConfirmView
  • BackupListView showing DangerousTipSheetView
  • ChildAccountDetailView showing UnlinkConfirmView
  • DevicesInfoView showing DangerousTipSheetView
  • MultiBackupDetailView showing DangerousTipSheetView
  • NFTAddCollectionView showing NFTAddCollectionView.NFTCollectionEnableView
  • NFTDetailPage showing MoveSingleNFTView
  • StakeAmountView showing StakeConfirmView
  • SwapView showing SwapConfirmView
  • TokenDetailView showing MoveTokenView
  • WalletHomeView showing BackupTipsView
  • WalletSendAmountView showing SendConfirmView

Views using the old PresentHostingController (in RouteMap):

  • ImportAccountsView
  • MoveNFTsView
  • MoveAccountsView
  • AddTokenSheetView
  • AccountSwitchView
  • BrowserAuthnView
  • BrowserAuthzView
  • BrowserSignMessageView
  • BrowserSignTypedMessageView

Screenshots (if applicable)

N/A

- Refactored to work with SwiftUI views presented as sheets, and SwiftUI views wrapped in view controllers and presented via navigation controllers
…w dynamic sheets

- Reverted some of the views to `PresentHostingController` because using scroll views, not compatible with dynamic heights due to the lack of intrinsic size
@jeden jeden changed the title Feature/647 refactor pull up sheets 647 Refactor pull-up sheets to use dynamic height Dec 19, 2024
@zhouxl
Copy link
Collaborator

zhouxl commented Dec 19, 2024

Have you tried the effect?

@jeden jeden linked an issue Dec 19, 2024 that may be closed by this pull request
@Outblock Outblock deleted a comment from hellolighten-team Dec 20, 2024
@jeden
Copy link
Contributor Author

jeden commented Dec 20, 2024

Have you tried the effect?

Which effect are you referring to?

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.

[TECH-DEBT] Refactor pull-up sheets to use dynamic height
2 participants