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(ConnectDAppModal): add scrolling #15847

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

caybro
Copy link
Member

@caybro caybro commented Jul 29, 2024

What does the PR do

  • wrap the contents into a scroll view and don't hardcode the height

Fixes #15592

Affected areas

ConnectDAppModal

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it
Zaznam.obrazovky.z.2024-07-29.10-43-51.webm

- wrap the contents into a scroll view and don't hardcode the height

Fixes #15592
@status-im-auto
Copy link
Member

status-im-auto commented Jul 29, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 100c048 #1 2024-07-29 08:51:35 ~7 min tests/nim 📄log
✔️ 100c048 #1 2024-07-29 08:55:24 ~10 min macos/x86_64 🍎dmg
✔️ 100c048 #1 2024-07-29 08:58:06 ~13 min tests/ui 📄log
✔️ 100c048 #1 2024-07-29 08:58:21 ~13 min macos/aarch64 🍎dmg
✔️ 100c048 #1 2024-07-29 09:00:05 ~15 min linux-nix/x86_64 📦tgz
✔️ 100c048 #1 2024-07-29 09:02:11 ~17 min linux/x86_64 📦tgz
✔️ 100c048 #1 2024-07-29 09:20:19 ~35 min windows/x86_64 💿exe

@status-im-auto
Copy link
Member

Copy link
Contributor

@Seitseman Seitseman left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -93,56 +91,64 @@ StatusDialog {
signal disconnect()

width: 480
implicitHeight: !d.connectionAttempted ? 633 : 681
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on ConnectING UI and ConnectED UI
maybe it would make sense to leave implicitHeight with those values as is?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, no hardcoded popup heights; this allows the popup to freely shrink and grow (within the margins limits)

@Seitseman
Copy link
Contributor

Should it be merged into 2.30 release branch?

Copy link
Contributor

@alexjba alexjba left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 Tested

@caybro caybro merged commit 0e0b264 into master Jul 29, 2024
9 checks passed
@caybro caybro deleted the 15592-fix-dapps-connect-scolling branch July 29, 2024 11:04
@caybro
Copy link
Member Author

caybro commented Jul 29, 2024

Should it be merged into 2.30 release branch?

Yeah, created the backport PR: #15854

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.

dApp - sd-6 - Unable to read full contents of connection request dialog
4 participants