-
Notifications
You must be signed in to change notification settings - Fork 76
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: Adding Beta user agreement #11100
fix: Adding Beta user agreement #11100
Conversation
Jenkins BuildsClick to see older builds (19)
|
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.
Nice :)
66997aa
to
d208584
Compare
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.
Please add proper sizing to text component (like Layout.fillWidth: true
) to have proper layout no matter how long text we have.
Here the text is extended on purpose, similar situation may happen for translations.
Btw. things like that can be easily check with the inspection tool (but it's temporarily broken for popups - #11111)
|
||
UserAgreementPopup { | ||
id: popup | ||
visible: true |
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.
please add for convenience:
anchors.centerIn: parent
modal: false
d208584
to
a5fc1c0
Compare
Fixed. I've added wordWrap to the text paragraph and Layout.fillWidth to the checkboxes. Looks good now with large text. |
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.
Nice work! Some minor comments left, but nothing serious.
Layout.preferredWidth: 64 | ||
Layout.preferredHeight: 64 | ||
Layout.alignment: Qt.AlignHCenter | ||
image.source: Style.png("status-logo") |
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.
adding image.mipmap: true
here improves quality a bit bc the original image is much bigger than 64x64
Layout.fillWidth: true | ||
Layout.topMargin: -10 //reduced margin by design | ||
Layout.bottomMargin: layout.spacing | ||
contentItem: Paragraph { |
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.
maybe
Layout.fillWidth: true
Layout.fillHeight: true
could be moved from Paragraph
to AgreementSection
bc in this context those attached properties make no sense.
storybook/figma.json
Outdated
@@ -212,5 +212,8 @@ | |||
"https://www.figma.com/file/17fc13UBFvInrLgNUKJJg5/Kuba%E2%8E%9CDesktop?type=design&node-id=29528%3A588403&t=GUxMZEWcfRO7pXJb-1", | |||
"https://www.figma.com/file/17fc13UBFvInrLgNUKJJg5/Kuba%E2%8E%9CDesktop?type=design&node-id=29528%3A588563&t=GUxMZEWcfRO7pXJb-1", | |||
"https://www.figma.com/file/17fc13UBFvInrLgNUKJJg5/Kuba%E2%8E%9CDesktop?type=design&node-id=29528%3A587660&t=GUxMZEWcfRO7pXJb-1" | |||
], | |||
"UserAgreementPopup": [ | |||
"https://www.figma.com/file/17fc13UBFvInrLgNUKJJg5/Kuba⎜Desktop?type=design&node-id=1023-298171&t=Q4MOViPCoHsTjhs6-0" |
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.
a5fc1c0
to
822c85d
Compare
822c85d
to
dfc9f7e
Compare
What does the PR do
Closing: #11044
Changes:
Affected areas
AppMain
Screenshot of functionality (including design for comparison)