-
Notifications
You must be signed in to change notification settings - Fork 81
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
Conversation
- wrap the contents into a scroll view and don't hardcode the height Fixes #15592
Jenkins Builds
|
There was a problem hiding this 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
Should it be merged into 2.30 release branch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍 Tested
Yeah, created the backport PR: #15854 |
What does the PR do
Fixes #15592
Affected areas
ConnectDAppModal
Screenshot of functionality (including design for comparison)
Zaznam.obrazovky.z.2024-07-29.10-43-51.webm