-
Notifications
You must be signed in to change notification settings - Fork 581
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
LineEdit: fixes cursor draws out of bounds #6382
Conversation
} | ||
|
||
min-height: i-text-input.preferred-height; | ||
min-width: max(50px, i-placeholder.min-width); |
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.
I'm guessing now, but... was this line removed because the real LineEdit
components end up with a bigger minimum anyway?
2a06e96
to
912687f
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.
This seems to work - great. I guess the actual fix was to move the LineEditBase
to be a child of the border Rectangle
instead of overlay?
Please also see the comment inline. max(1px)
seems odd to me :)
// min-width < 1px could cause trouble on focus | ||
min-width: max(1px); |
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.
Why use max(1px)
instead of 1px
? Could you also please extend the comment to elaborate on what "trouble on focus" means? How could the min-width become zero?
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.
This is indeed wierd. I think the focus problem appear if the FocusScope or whatever is being clipped has a size of 0, which shouldn't happen. I haven't tried this change, bur I'm afraid that it does something fishy with the default sizing of the LineEdit
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.
Why use
max(1px)
instead of1px
? Could you also please extend the comment to elaborate on what "trouble on focus" means? How could the min-width become zero?
Thanks the max thing was just in my head and should not be there. Is removed.
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.
This is indeed wierd. I think the focus problem appear if the FocusScope or whatever is being clipped has a size of 0, which shouldn't happen. I haven't tried this change, bur I'm afraid that it does something fishy with the default sizing of the LineEdit
Yeah it is strange. But should we nevertheless merge this, because it fixes a bug? I have tried this pr with the example described in the issue and also with different styles and with different width settings, it looks like it works fine. What do you think @ogoffart ?
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.
Could you also please extend the comment to elaborate on what "trouble on focus" means? How could the min-width become zero? What caused you to modify this line?
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.
I have edited the comment. It could be 0 because of how a developer uses it from outside. He can define e.g. the width or min-width of a LineEdit
. What also causes the problem. @tronical
Fixes #6243