Constrain button insets to gtx.Constraints.

Signed-off-by: Larry Clapp <larry@theclapp.org>
---
 widget/material/button.go | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/widget/material/button.go b/widget/material/button.go
index ba9c8e6..ad78bb8 100644
--- a/widget/material/button.go
+++ b/widget/material/button.go
@@ -60,7 +60,9 @@ func (b Button) Layout(gtx *layout.Context, button *widget.Button) {
 	col := b.Color
 	bgcol := b.Background
 	hmin := gtx.Constraints.Width.Min
+	hmax := gtx.Constraints.Width.Max
 	vmin := gtx.Constraints.Height.Min
+	vmax := gtx.Constraints.Height.Max
 	layout.Stack{Alignment: layout.Center}.Layout(gtx,
 		layout.Expanded(func() {
 			rr := float32(gtx.Px(unit.Dp(4)))
@@ -80,9 +82,23 @@ func (b Button) Layout(gtx *layout.Context, button *widget.Button) {
 			gtx.Constraints.Width.Min = hmin
 			gtx.Constraints.Height.Min = vmin
 			layout.Center.Layout(gtx, func() {
-				layout.Inset{Top: unit.Dp(10), Bottom: unit.Dp(10), Left: unit.Dp(12), Right: unit.Dp(12)}.Layout(gtx, func() {
+				// Draw the label into a macro to get its dimensions, so we can
+				// constrain the inset.
+				var m op.MacroOp
+				m.Record(gtx.Ops)
+				widget.Label{}.Layout(gtx, b.shaper, b.Font, b.TextSize, b.Text)
+				m.Stop()
+				labelDims := gtx.Dimensions
+
+				// Constrain the inset to the outer max h/v, minus the label size
+				hInset := unit.Px(float32(min(gtx.Px(unit.Dp(24)), hmax-labelDims.Size.X)) / 2)
+				vInset := unit.Px(float32(min(gtx.Px(unit.Dp(20)), vmax-labelDims.Size.Y)) / 2)
+
+				// Finally, draw the label inside the inset.
+				layout.Inset{Top: vInset, Bottom: vInset, Left: hInset, Right: hInset}.Layout(gtx, func() {
 					paint.ColorOp{Color: col}.Add(gtx.Ops)
-					widget.Label{}.Layout(gtx, b.shaper, b.Font, b.TextSize, b.Text)
+					m.Add()
+					gtx.Dimensions = labelDims
 				})
 			})
 			pointer.Rect(image.Rectangle{Max: gtx.Dimensions.Size}).Add(gtx.Ops)
@@ -91,6 +107,13 @@ func (b Button) Layout(gtx *layout.Context, button *widget.Button) {
 	)
 }
 
+func min(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}
+
 func (b IconButton) Layout(gtx *layout.Context, button *widget.Button) {
 	layout.Stack{}.Layout(gtx,
 		layout.Expanded(func() {
-- 
2.25.0

On Thu Feb 13, 2020 at 09:53, Larry Clapp wrote:
> Constrain button insets to gtx.Constraints.
>

Thanks. Why not adjust insets in layout.Insets directly?

You mean, like, pass inset values (or maybe just an Inset object) into
the Layout function?  That seemed pretty intrusive.  Also, I know
we're trying to conform to the Material Design spec (which I think is
a good idea, in general), and I assumed that the insets you'd
hardcoded conformed to that spec, and (in general) shouldn't be
changed.  But I also wanted a shorter button, so I made it care about
the constraints I gave it.

If you meant something else, well, please elaborate.  :)

— L

On Thu, Feb 13, 2020 at 12:07 PM Elias Naur <mail@eliasnaur.com> wrote:
>
> On Thu Feb 13, 2020 at 09:53, Larry Clapp wrote:
> > Constrain button insets to gtx.Constraints.
> >
>
> Thanks. Why not adjust insets in layout.Insets directly?

On Thu Feb 13, 2020 at 12:39, Larry Clapp wrote:
> You mean, like, pass inset values (or maybe just an Inset object) into
> the Layout function?  That seemed pretty intrusive.  Also, I know
> we're trying to conform to the Material Design spec (which I think is
> a good idea, in general), and I assumed that the insets you'd
> hardcoded conformed to that spec, and (in general) shouldn't be
> changed.  But I also wanted a shorter button, so I made it care about
> the constraints I gave it.
>
> If you meant something else, well, please elaborate.  :)
>

Sorry for not being clear. I'm wondering whether it would be useful to add this
inset squeezing logic to the layout.Inset.Layout method? More precisely,
layout.Inset can be changed to cope with its child widget overflowing the inset
constraints, by adjusting the final TransformOp at

	https://git.sr.ht/~eliasnaur/gio/tree/master/layout/layout.go#L125

the same way you're doing it in your patch.

The advantage would be a more robust layout.Inset and simpler code in
Button.Layout.

-- elias

On Fri, Feb 14, 2020 at 8:07 AM Elias Naur <mail@eliasnaur.com> wrote:
> Sorry for not being clear. I'm wondering whether it would be useful to add this
> inset squeezing logic to the layout.Inset.Layout method? More precisely,
> layout.Inset can be changed to cope with its child widget overflowing the inset
> constraints, by adjusting the final TransformOp

So I tried this, and maybe I did it wrong, but I couldn't get it to work.

It turns out that a lot of widgets will take whatever space you give
them (reasonably enough), so unless you reduce the constraint by the
insets, they'll take up that space.

And if you *do* reduce the constraints by the insets, then it's
impossible for the widget to take up more space than that, so there's
no point in reducing the insets, because of course there's space for
them.

You're welcome to give it a shot, but I couldn't get it to work.
(Though again, maybe I did it wrong.)  My buttons in my app were fine,
but the "kitchen" example insisted on drawing flush-left with 0 inset.

— L

On Sat Feb 15, 2020 at 12:40, Larry Clapp wrote:
> On Fri, Feb 14, 2020 at 8:07 AM Elias Naur <mail@eliasnaur.com> wrote:
> > Sorry for not being clear. I'm wondering whether it would be useful to add this
> > inset squeezing logic to the layout.Inset.Layout method? More precisely,
> > layout.Inset can be changed to cope with its child widget overflowing the inset
> > constraints, by adjusting the final TransformOp
>
> So I tried this, and maybe I did it wrong, but I couldn't get it to work.
>
> It turns out that a lot of widgets will take whatever space you give
> them (reasonably enough), so unless you reduce the constraint by the
> insets, they'll take up that space.
>
> And if you *do* reduce the constraints by the insets, then it's
> impossible for the widget to take up more space than that, so there's
> no point in reducing the insets, because of course there's space for
> them.
>
> You're welcome to give it a shot, but I couldn't get it to work.
> (Though again, maybe I did it wrong.)  My buttons in my app were fine,
> but the "kitchen" example insisted on drawing flush-left with 0 inset.
>

I see now why my idea wasn't any good, sorry for wasting your time.

However, I'm still not convinced your approach is a good idea. For example,
your method will not support button labels that otherwise fall back to multiple
lines if insets were preserved (e.g. a "Delete now" that displays as
"Delete\nnow" on two lines when squeezed).

How about adding an Insets field to Button, initialized by the
material.Theme.Button method, and honored in material.Button.Layout? You can
then tweak the insets (possibly to 0) before calling Layout.

-- elias

On Sat, Feb 15, 2020 at 2:08 PM Elias Naur <mail@eliasnaur.com> wrote:
> I see now why my idea wasn't any good, sorry for wasting your time.

No worries.

> However, I'm still not convinced your approach is a good idea. For example,
> your method will not support button labels that otherwise fall back to multiple
> lines if insets were preserved (e.g. a "Delete now" that displays as
> "Delete\nnow" on two lines when squeezed).
>
> How about adding an Insets field to Button, initialized by the
> material.Theme.Button method, and honored in material.Button.Layout? You can
> then tweak the insets (possibly to 0) before calling Layout.

That does seem to work.  See new patch "allow Button Inset to be customizable".

That said, this still doesn't automatically adjust a button's insets
given its constraints, but at least it makes it *possible* to do it
manually.

Thanks for your help.

— L

On Sat Feb 15, 2020 at 3:49 PM, Larry Clapp wrote:
> On Sat, Feb 15, 2020 at 2:08 PM Elias Naur <mail@eliasnaur.com> wrote:
> > I see now why my idea wasn't any good, sorry for wasting your time.
>
> 
> No worries.
>
> 
> > However, I'm still not convinced your approach is a good idea. For example,
> > your method will not support button labels that otherwise fall back to multiple
> > lines if insets were preserved (e.g. a "Delete now" that displays as
> > "Delete\nnow" on two lines when squeezed).
> >
> > How about adding an Insets field to Button, initialized by the
> > material.Theme.Button method, and honored in material.Button.Layout? You can
> > then tweak the insets (possibly to 0) before calling Layout.
>
> 
> That does seem to work. See new patch "allow Button Inset to be
> customizable".
>

It appears the patch didn't get through. Can you please resend the patch
to my address? I'm very sorry for the inconvenience.

	https://todo.sr.ht/~sircmpwn/lists.sr.ht/138