Add support for Wrapping for RadioGroup option strings#6369
Add support for Wrapping for RadioGroup option strings#6369qiulaidongfeng wants to merge 10 commits into
Conversation
andydotxyz
left a comment
There was a problem hiding this comment.
Thanks for looking at this. It is too late for 2.8 unfortunately as we have passed feature freeze, but if you target 2.9 we can land it after release.
It is worrying that there are so many layout changes for an optional new parameter - can you comment on that?
A few inline comments about things that don't seem quite right too.
| test.WithTestTheme(t, func() { | ||
| render.Refresh() | ||
| customTextSize = render.label.TextSize | ||
| customTextSize = render.label.Theme() |
There was a problem hiding this comment.
This is because previously we set txt.TextSize = i.Theme().Size(theme.SizeNameText) in radioItemRenderer.Refresh, but RichText does not have this field. so my do r.label.Segments[0].(*TextSegment).Style.SizeName = theme.SizeNameText,but it not change Size() result. If there is a better way to do this, please let me know. Thank you
There was a problem hiding this comment.
Sorry, I meant more literally - it makes no sense to have a fyne.Theme that is called customTextSize.
There was a problem hiding this comment.
If you want to check the text size output for a widget you can use WidgetRenderer(...) to get the renderer and then call Objects() on that to get the canvas.Text.
|
|
||
| func newRadioItem(label string, onTap func(*radioItem)) *radioItem { | ||
| i := &radioItem{Label: label, onTap: onTap} | ||
| func newRadioItem(label string, Wrapping fyne.TextWrap, onTap func(*radioItem)) *radioItem { |
There was a problem hiding this comment.
It looks like this does not propagate change if Wrapping is set after initial creation
| } | ||
| var itemHeight, itemWidth float32 | ||
| minSize := r.radio.MinSize() | ||
| minSize := s |
There was a problem hiding this comment.
This looks incorrect - the minSize of an object should not be set to its current size
There was a problem hiding this comment.
I revised the code to demonstrate my intention. Specifically, when RichText uses fyne.TextWrapBreak, its MinSize might not occupy enough screen space, resulting in unexpected line breaks.
|
Thanks for your quick reply.
This is inevitable as radioItemRenderer has been changed from using canvas.Text to widget.RichText. func newRadio(options []string, onclick func(string)) (*fyne.Container, func(tf bool, sigle int, typ int)) {
var selected string
var checkboxes []*widget.Check
content := container.NewVBox()
for i, option := range options {
label := widget.NewLabel(option)
label.Wrapping = fyne.TextWrapBreak
check := widget.NewCheck("", func(checked bool) {
if checked {
for j, cb := range checkboxes {
if j != i {
cb.SetChecked(false)
} else {
selected = options[j]
onclick(selected)
}
}
} else {
selected = ""
}
})
checkboxes = append(checkboxes, check)
hbox := container.NewVBox(check, label)
content.Add(hbox)
}
return content, func(b bool, single int, typ int) {
if typ == 0 {
if b {
checkboxes[0].SetChecked(true)
} else {
checkboxes[1].SetChecked(true)
}
return
}
checkboxes[single].SetChecked(true)
}
}This is just to make text display using the existing fyne.TextWrapBreak. Secondly, this is similar to word-break: break-all and only requires one line, which is easy to learn. |
andydotxyz
left a comment
There was a problem hiding this comment.
With this the type of Wrapping is now different for RadioItem vs RadioGroup - inconsistency is bad.
I think you'll need a different approach to updating (typically a render retains a reference to the type it renders, perhaps that works here?
andydotxyz
left a comment
There was a problem hiding this comment.
Just a couple of comments inline.
However the big issue here is what you see in all the test changes - the widget is no longer the same size. Moving text -> RichText does make sizing changes but they need to be handled internally. I suspect your usage of InlineIconSize are trying to approximate, but it looks wrong.
| test.WithTestTheme(t, func() { | ||
| render.Refresh() | ||
| customTextSize = render.label.TextSize | ||
| customTextSize = render.label.Theme() |
There was a problem hiding this comment.
Sorry, I meant more literally - it makes no sense to have a fyne.Theme that is called customTextSize.
| test.WithTestTheme(t, func() { | ||
| render.Refresh() | ||
| customTextSize = render.label.TextSize | ||
| customTextSize = render.label.Theme() |
There was a problem hiding this comment.
If you want to check the text size output for a widget you can use WidgetRenderer(...) to get the renderer and then call Objects() on that to get the canvas.Text.
add field Wrapping to RadioGroup.
In order to enable word wrapping
for text exceeding the screen
when creating a single choice question UI,
similar to the TextWrapBreak for widget.Label
Fixes #5656