fix(completionentry): calculate max size and resize correctly #95
Conversation
The new max size calculation ensures the popup position is always below the entry widget. The addition of the Resize method ensures the popup is updated with the rest of the widget upon resize. Since now the popup doesn't go above the entry widget not all options are rendered and the test had to be updated to select the first item as the other ones are not visible
geodimm
force-pushed
the
fix-completionentry-resize
branch
from
233b279 to
27ea5b9
Compare
widget/completionentry.go
Outdated
| @@ -136,7 +146,8 @@ type navigableList struct { | |||
| } | |||
|
|
|||
| func newNavigableList(items []string, entry *widget.Entry, setTextFromMenu func(string), hide func(), | |||
| create func() fyne.CanvasObject, update func(id widget.ListItemID, object fyne.CanvasObject)) *navigableList { | |||
| create func() fyne.CanvasObject, update func(id widget.ListItemID, object fyne.CanvasObject), | |||
There was a problem hiding this comment.
I don't understand this change - it looks like you have added another item to the parameters to put it on a new line?
There was a problem hiding this comment.
It's just my formatter automatically formatting the source code, it's a non-functional change :)
There was a problem hiding this comment.
@andydotxyz Would you prefer to undo the white space change or you're happy to merge the PR as is?
There was a problem hiding this comment.
Yes, please do put it back to not having an empty parameter which is just newline.
I will get our CI updated so that the linter runs correctly for our code standards.
There was a problem hiding this comment.
@andydotxyz would it be possible to get this merged soon? 🙏
Pull Request Test Coverage Report for Build 12331391666Details
💛 - Coveralls |
|
I wonder if there maybe should be a follow-up for when the select is at the bottom of a screen and should pop above instead of below? Perhaps there should be a minimum size check and if the "down" is too short use "up" instead? |
|
Yes, I was also thinking about making the size configurable, e.g. for the dropdown to take 50% or 75% of the screen size as opposed to filling it. Both would be good improvements |
|
A word of warning - don't make widget size relative to screen/window size - it leads to bad outcomes. If you want to configure how much content should be displayed then it should be relative to some content reasoning not window space. |
The new max size calculation ensures the popup position is always below
the entry widget.
The addition of the Resize method ensures the popup is updated with the
rest of the widget upon resize.
Since now the popup doesn't go above the entry widget not all options
are rendered and the test had to be updated to select the first item as
the other ones are not visible
Before:
https://github.com/user-attachments/assets/baff9482-3543-4e8a-8e52-cdad1ef79dcc
After:
https://github.com/user-attachments/assets/981f6a41-7705-42d0-a075-555c65fc28df