gui icon indicating copy to clipboard operation
gui copied to clipboard

[WIP] added layout basics

Open Keitio opened this issue 6 years ago • 16 comments

Definitely don't merge this. This is an attempt of mine at creating a layout system for this GUI system. This is largely uncommented code, with some bits taken from the examples. My take is that the layouts take an Env as parameter, and give back a map of string to Env. The map keys are like "tags" for the sub Envs. You could have a Scroller with "body", "header" and "footer" areas for example. The Layouts are then basically acting as sophisticated Muxes.

If this idea takes on, some refactoring is needed because a lot of code for Layout is pasted from Mux.

Keitio avatar Jul 04 '19 14:07 Keitio

Hey, thanks for this! This is definitely a step in the right direction, even if not polished. Sorry for not responding earlier.

If you wish to finish this into something that could be merged, I'll gladly provide code reviews.

The idea of returning a map is interesting. What would be the advantages of this over simply returning a slice of Envs?

faiface avatar Jul 07 '19 10:07 faiface

Actually it was a slice of Envs originally, but it was a mess to manage. For example, how do you address a grid ? you would need to remember the row length to address it. Documentation can also be clearer this way, rather than saying "1 is the main body". I'm working on a second revision with a Zone interface, which is just a String() string, used as map keys, which would make it even clearer. I'll try to push it tomorrow or the day after that.

Keitio avatar Jul 07 '19 11:07 Keitio

Actually it was a slice of Envs originally, but it was a mess to manage. For example, how do you address a grid ?

Good point, but how often do you need to address it? Don't you just create elements and let it go? I'm not arguing, just wanna get more info on this :D

faiface avatar Jul 07 '19 11:07 faiface

You're right, we only address them once, but it's mainly for clarity purposes. For simple layouts, like a grid or a list, a slice is perfectly fine. But if you take a more complex layout, like a Card, using title and image is way clearer than 1 and 0 for me. This way you can know what your code is doing at a glance.

Keitio avatar Jul 07 '19 11:07 Keitio

What about something like this?

var left, middle, right gui.Env
hsplit := layout.NewHSplit(env, &left, &middle, &right)

And this

var (
    topLeft, top, topRight          gui.Env
    left, middle, right             gui.Env
    bottomLeft, bottom, bottomRight gui.Env
)

grid := layout.NewGrid(
    env,
    []*gui.Env{
    	{&topLeft, &top, &topRight},
    	{&left, &middle, &right},
    	{&bottomLeft, &bottom, &bottomRight},
    },
)

EDIT:

And this:

var title, image gui.Env
card := layout.NewCard(env, &layout.CardConfig{
    Title: &title,
    Image: &image,
})

faiface avatar Jul 07 '19 11:07 faiface

This is a great alternative i hadn't thought about, and it would make a lot of things easier. I'll implement that instead, thanks.

Keitio avatar Jul 07 '19 13:07 Keitio

Done, i also introduced a Box layout and a true layout.Mux. I think there is an abstraction to be found for merging gui.Mux and layout.Mux, and this still needs some documenting, but otherwise i think it's quite good for a start. I'm also unsure as how to do options for layouts correctly, as there is quite some boilerplate and typing involved.

Keitio avatar Jul 09 '19 09:07 Keitio

Great idea with the Layout interface!

I am quite busy right now (and gonna be tomorrow as well), but a few quick thoughts:

  1. It would make sense to make a SplitFunc type for splitting functions.
  2. I think the SplitFunc would be more flexible if it accepted not just a width, but minX and maxX. Otherwise you need to modify all returned rectangles.
  3. The usage of the Layout interface is kinda obfuscated right now. For example, NewGrid return gui.Env, but uses NewMux to do so, where NewMux accepts a Layout, which a *Grid is so it passes itself in there. How about NewGrid just returned a Layout? Then it would be quite easier to understand.
  4. Types like *Grid and *Box probably don't have to be exported.
  5. Yeah, I agree that the functional options make things a little verbose here. If the layout only accepts a few parameters (especially if they are distinguished by type), it would make sense to simply accept them as parameters.

Thanks for the great work so far, btw :)

faiface avatar Jul 10 '19 17:07 faiface

Thanks ! I'll create the SplitFunc type, as it will be more understandable and documentable this way. For its arguments, I think it's fine as-is, because you need to create Rectangles from ints anyway, so having to do less work function side is a win by me. For the NewGrid() using an layout.Mux internally, unfortunately I kind of need to do either that or add some kind of Init() func to the Layout interface. But I agree, it's very obfuscated right now. And you're right, I'll change the layouts to be unexported, exporting them only made sense when there was no constructor.

Keitio avatar Jul 11 '19 08:07 Keitio

I was also thinking of splitting the Layout interface with a Redrawer interface, that could be used for a widgets package in the future. This Redrawer would be in the gui package. Would that make any sense to you ?

Keitio avatar Jul 11 '19 08:07 Keitio

I refactored quite a bit and added an Items() []*gui.Env func to the Layout interface, this way this is less obfuscated and feels less like magic.

Keitio avatar Jul 12 '19 15:07 Keitio

Hm, I'm not sure I like the Items() method, it seems like a hack. I'll have to think more about this. If you come up with new ideas, please share!

I finally have more time again, so I'll be able to devote more time to this. I'll share new ideas as soon as I get them.

faiface avatar Jul 14 '19 18:07 faiface

I agree, it feels like a hack, but it felt less like a hack than some Init() func. It was more for lack of a better idea. I think we could add a parameter to NewMux, like that:

layout.NewMux(
	mux.MakeEnv(),
	[]*gui.Env{&top, &left, &right}, // !
	layout.NewGrid(
		[][]*gui.Env{
			{&top},
			{&left, &right},
		},
	),
)

And voilà, no need for magic. It feels repetitive, but nothing unbearable, especially if we provide helpers. Does this feel alright with you ?

There is still a problem though, because it's not really tied to the Layout. We would need to provide the Envs in the same order that the Lay() will give them, so we need to enforce some kind of ordering.

EDIT: we could even

layout.NewMux(
	mux.MakeEnv(),
	[]*gui.Env{&top, &left, &right},
	layout.NewGrid(
		[]int{ 1, 2 }, // for row length
	),
)

because the Envs values are not used inside the Layouts, and it would make the Layouts independent from gui.

Keitio avatar Jul 15 '19 08:07 Keitio

Well after implementing it it works really well, there's just something strange I can't quite put my finger on. Also I changed the initialization from functional options to struct literals with sane defaults. It's better documentable this way, and less contrived for simple cases.

Keitio avatar Jul 15 '19 13:07 Keitio

I found two major problems with this approach:

  • a Layout can't ask for a childs redraw by itself
  • a Layout can't intercept Events

The first thing that would come to mind is making the Layout some kind of Event middleware, with a Transform(ev <-chan Event) <-chan Event {} func. It would then be able to re-emit the last resize event for childs redrawing, and use Events for itself, for example some win.MoScroll for a scroll layout.

But if some better idea comes out, I'll take it for sure.

Keitio avatar Jul 16 '19 14:07 Keitio

I didn't get much time on this project lately, but here it is, mostly done and usable. If it gets merged into master, it should definitely be marked as experimental and subject to change (well, even more than the rest). The only major problem I found is a performance one on the Scroller layout, but it works now.

Keitio avatar Aug 07 '19 14:08 Keitio