Refactoring: internal widgets #875
Conversation
| } | ||
|
|
||
| b.hidden = false | ||
| w.Refresh() |
There was a problem hiding this comment.
how come show uses w.Refresh(), but hide uses canvas.Refresh(w)?
There was a problem hiding this comment.
That’s because a hidden widget won’t be rendered. Therefore w.Refresh() is a waste of time there.
| r.Layout(size) | ||
| } | ||
|
|
||
| func (b *base) show(w fyne.Widget) { |
There was a problem hiding this comment.
I was going to ask if show can be moved up next to hide, then I realized you have made all of these files declare members in alphabetical order and I think I prefer that, danke!
There was a problem hiding this comment.
Gern geschehen! (My pleasure!)
internal/widget/base_renderer.go
Outdated
| ) | ||
|
|
||
| // NewBaseRenderer creates a new BaseRenderer. | ||
| func NewBaseRenderer(objects []fyne.CanvasObject) BaseRenderer { |
| "fyne.io/fyne/theme" | ||
| ) | ||
|
|
||
| type baseRenderer struct { |
There was a problem hiding this comment.
I understand why this was moved into internal, however this convenience is no longer unaccessible for developers creating custom widgets. We could expose it as:
package widget
import "fyne.io/fyne/internal/widget"
type BaseRenderer struct {
widget.baseRenderer
}
What do you think?
There was a problem hiding this comment.
I think the advantage is too small to do so.
@andydotxyz convinced me to not add such a construct for the base widget where the advantage would be much bigger. Therefore I would not do that here and now.
There was a problem hiding this comment.
I think this was not available to any custom widget creation because it was not exposed. The conversation I had with @toaster largely came to the conclusion that someone writing a widget could be expected to write a complete renderer, they do not have the same inheritance properties that widget objects do. Additionally if we make it too "simple" or "static" then there is a much higher chance that third party widgets would not respond well on refresh or theme change (I guess?).
There is another discussion about making it easier to create widgets with trivial renderers at #709 but honestly I am coming to the view that such enablers might encourage things that we don't think are good ideas (such as realising that customisation is difficult resorting to everything becoming a clickable image using a png that does not reflect theme colours)...
It's probably quite a big discussion but given that this was not exposed we can always decide to add your proposed solution in the future.
Description:
This PR moves several internal widgets (which are needed for the upcoming menu refactoring) into a new internal
widgetpackage.Checklist:
[ ] Tests included.