Layout - discussion

[FedeClaudi]

Hi Will,

I'm loving the new Layout functionality, thought I'd drop a couple items for discussion.

1. layout.split() with no renderable.

Currently if you are splitting layouts to create your structure, you will do something like:

layout.split(Layout())

The inner call to layout feels a bit cumbersome: if you are splitting a layout and not passing a renderable it could infer that it needs to spawn a new layout (and pass any **kwargs to it), thus making the sintax a bit simpler.

2. layout.split could return a copy of the new layout
   Currently the syntax looks like this:

layout.split(Layout(name='header'))
layout['header'].update(Panel)

but if split returned a pointer to the new layout, you could get:

layout.split(Layout(name='header')).update(Panel)

It's two small changes, but I feel like together they would make the code more concise and cleaner.
In an application of mine I'd go from:

        layout.split(Layout(name='head', ratio=2, size=6))
        layout.split(Layout(name='body', ratio=5, direction='horizontal'))
        layout.split(Layout(name='foot', ratio=1, size=4))

        # make header
        title_panel, info_panel = self.make_header()
        layout['head'].split(Layout(name='H-l'), direction='horizontal')
        layout['head']['H-l'].update(title_panel)
        layout['head'].split(Layout(name='H-r'), direction='horizontal')
        layout['head']['H-r'].update(info_panel)

        # make body
        layout['body'].split(Layout(name='bl'))
        layout['body'].split(Layout(name='br', ratio=2))

        layout['body']['br'].split(Layout(name='brt'))
        layout['body']['br'].split(Layout(name='brb', ratio=2))
        
        layout['body']['br']['brt'].update(Panel(Trajectory(), border_style=salmon, box=box.SIMPLE, title='trajectory'))
        layout['body']['br']['brb'].update(Panel(self.make_logger(), style=indigo, title="LOG"))

        # make body left
        layout['body']['bl'].split(Layout(name='blt'))
        layout['body']['bl'].split(Layout(name='blm', ratio=2))
        layout['body']['bl'].split(Layout(name='blb'))

        layout['body']['bl']['blt'].update(Panel(
                    variables, style=orange, padding=(1, 3), title="Variables"
                ))
        layout['body']['bl']['blm'].update(Panel(state, style=green, title="Physics"))
        layout['body']['bl']['blb'].update(Panel(StackInfo(), title="Call stack", box=box.SIMPLE_HEAD))

        # make footer
       layout['foot'].update(self.footer())

to

    layout.split(name='head', ratio=2, size=6)
    layout.split(name='body', ratio=5, direction='horizontal')
    layout.split(ratio=1, size=4).update(self.footer())

    # make header
    title_panel, info_panel = self.make_header()
    layout['head'].split(direction='horizontal').update(title_panel)
    layout['head'].split(direction='horizontal').update(info_panel)

    # make body
    layout['body'].split(name='bl')
    layout['body'].split(name='br', ratio=2)

    layout['body']['br'].split().update(
            Panel(Trajectory(), border_style=salmon, box=box.SIMPLE, title='trajectory'))
    layout['body']['br'].split(ratio=2).update(
        Panel(self.make_logger(), style=indigo, title="LOG"))
    
    # make body left
    layout['body']['bl'].split().update(Panel(
                variables, style=orange, padding=(1, 3), title="Variables"
            ))
    layout['body']['bl'].split(ratio=2).update(Panel(state, style=green, title="Physics"))
    layout['body']['bl'].split().update(Panel(StackInfo(), title="Call stack", box=box.SIMPLE_HEAD))

(Note that the fact that you can do layout.split().update() means you can also avoid having to specify a name for leaf nodes.

[willmcgugan]

Hi Federico,

It's nice to have someone take an interest in the API!

I changed the split method recently. Previously it was exactly like your last example.

The reason I didn't like that is that the first call to split didn't actually split anything. You would would have a layout with a single single sub-layout (i.e. visually nothing split). That's more English semantics than anything technical, but it bothered me enough to explore an alternative.

I changed it so that split would accept multiple layout instances. I think you may have missed this from looking at the code, but that's the reason why it doesn't return anything - because there may be multiple layouts.

So to compare old api with new api:

layout.split(name='head', ratio=2, size=6)
layout.split(name='body', ratio=5, direction='horizontal')
layout.split(ratio=1, size=4).update(self.footer())

layout.split(
    Layout(name="head", ratio=2, size=6),
    Layout(name="body", ratio=5, direction="horizontal),
    Layout(self.footer(), ratio=1, size=4)
)

Granted, the second is a little more verbose. But I think it more explicitly indicates how things are nested.

That may be a personal preference, but it also has a more objective benefit in that it makes it possible to extend it with other Layout classes. There could be a GridLayout for example, or TabbedLayout.

BTW the name index on Layout objects works recursively. So layout['bl'] would work even it is not on the first level.

With that in mind, your code becomes something like this:

layout.split(
    Layout(name='head', ratio=2, size=6),
    Layout(name='body', ratio=5, direction='horizontal'),
    Layout(self.footer(), ratio=1, size=4)
)

# make header
title_panel, info_panel = self.make_header()
layout['head'].split(
    Layout(title_panel, direction='horizontal'),
    Layout(info_panel, direction='horizontal')
)

# make body
layout['br'].update(
    Panel(Trajectory(), border_style=salmon, box=box.SIMPLE, title='trajectory')
)
layout['br'].split(
    Layout(
        Panel(self.make_logger(), style=indigo, title="LOG")
        ratio=2
    )
)

layout['bl'].split(
    Layout(
        Panel(
            variables, style=orange, padding=(1, 3), title="Variables"
        )
    ),
    Layout(        
        Panel(state, style=green, title="Physics"),
        ratio=2
    ),
    Layout(
        Panel(StackInfo(), title="Call stack", box=box.SIMPLE_HEAD)  
    )
)

More verbose, but I think easier to scan.

Thoughts?

[FedeClaudi]

I think I messed up something in the way I've been getting the latest version of the layout code. I originally had a version which used this latest syntax, but then when I forked rich that stopped working and I had to change to the one I've posted above. My bad.

I will try again with the latest code and report back.

By the way, if at any point I'm overstepping with suggesting changes or comments on the API please do let me know, I just really enjoy using rich and as a (non-professional) developer I always feel like I don't get enough feedback about my projects, so I try to give some where I can.

[willmcgugan]

No worries, let me know how you get on.

I'm totally fine with bouncing ideas. It can only lead to improvements to the project!