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/dapps fixes #1330

Merged
merged 14 commits into from
Jan 14, 2025
Merged

Fix/dapps fixes #1330

merged 14 commits into from
Jan 14, 2025

Conversation

svojsu
Copy link
Contributor

@svojsu svojsu commented Jan 13, 2025

SUMMARY

There are few thing needed to be fixed and added for preparing Seamless DApp feature. Most of them are just layout and sorting/filtering fixes.

  • Added banner section for rebuilt UI of DAppList module.
  • Reused same dialog logic for both closing browser widget from minimized state and closing from Tab Manager screen.
  • Removed categories filtering logic from DAppSearch by query input.
  • Made favorites part of filtering logic for DAppSearch.
  • Fixed NovaMainAppContainer layout so it no longer breaks during screen rotations with device orientation changes.
  • Fixed DApp matching when adding to favorites.
  • Introduced DAppIconViewModelFactory to handle DApp icon and favicon logic.

SOLUTION SCREENSHOTS

Image name Image name Image name Image name Image name

@svojsu svojsu marked this pull request as ready for review January 13, 2025 16:15
@svojsu svojsu requested a review from ERussel January 13, 2025 16:38
let title: String
let subtitle: String

let imageViewModel: StaticImageViewModel
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't it be ImageViewModelProtocol?

)
else { return nil }

let domain = urlString[range]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks complicated. What is wrong with pageURL.host?

Copy link
Contributor Author

@svojsu svojsu Jan 14, 2025

Choose a reason for hiding this comment

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

pageURL.host might return nil for complex domains that contains subdomains i.e. app.parallel.finance

@@ -162,12 +193,12 @@ extension NovaMainAppContainerViewController {
rootView.addSubview(bottomView)

let topContainerBottomOffset = Constants.topContainerBottomOffset(for: view)
let minimizedWidgetHeight = Constants.minimizedWidgetHeight(for: view)
let widgetTopConstraintInset = UIScreen.main.bounds.height
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use bottom of the container itself instead of height of the screen? Feel not comfortable with assumptions about screen height

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just came up with better approach. Gonna update soon.

@svojsu svojsu merged commit 24b3bd1 into base/seamless-dapp-browser Jan 14, 2025
1 check failed
@svojsu svojsu deleted the fix/dapps-fixes branch January 14, 2025 11:22
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