Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

child property as second parameter #265

Merged
merged 11 commits into from
Jan 31, 2024
Merged

child property as second parameter #265

merged 11 commits into from
Jan 31, 2024

Conversation

Aylur
Copy link
Owner

@Aylur Aylur commented Jan 19, 2024

Example Media widget currently looks like this

function Media() {
    // declare components
    // and then return the layout
    return Widget.Box({
        class_name: 'player',
        children: [
            img,
            Widget.Box({
                vertical: true,
                hexpand: true,
                children: [
                    Widget.Box({
                        children: [
                            title,
                            icon,
                        ],
                    }),
                    artist,
                    Widget.Box({ vexpand: true }),
                    positionSlider,
                    Widget.CenterBox({
                        start_widget: positionLabel,
                        center_widget: Widget.Box({
                            children: [
                                prev,
                                playPause,
                                next,
                            ],
                        }),
                        end_widget: lengthLabel,
                    }),
                ],
            }),
        ],
    });
}

With this PR its possible to save some indents, and less child and children keyword

function Media() {
    return Widget.Box(
        { class_name: 'player' },
        img,
        Widget.Box(
            {
                vertical: true,
                hexpand: true,
            },
            Widget.Box({},
                title,
                icon,
            ),
            artist,
            Widget.Box({ vexpand: true }),
            positionSlider,
            Widget.CenterBox({
                start_widget: positionLabel,
                center_widget: Widget.Box({},
                    prev,
                    playPause,
                    next,
                ),
                end_widget: lengthLabel,
            }),
        ),
    );
}

two issues that arise with this

  1. the object parameter can feel wrong
Widget.Box(
    { // this gets pushed one indent deeper
        vertical: true,
        hexpand: true,
    },
    Widget.Box({},
        title,
        icon,
    ),
)

this could be mitigated with custom formatters

  1. the object parameter will have to be passed even when empty
Widget.Box({},
    title,
    icon,
)

this can be solved somewhat redundantly, but honestly I don't think we need to, we can just assume there will always be an object passed, only time I can imagine this happen is with Box, which should instead also take an array of widgets as its first parameter, just like how Label can take a string, but thats going to be another commit

edit: both are valid

Widget.Box({},
    title,
    icon,
)

Widget.Box([
    title,
    icon,
])

3. Widgets not fully subclassed, but only with Widget.subclass utility, will be typed as if they can take a child
3. Widgets not fully subclassed can't take a child as second parameter for simplicity

@Aylur Aylur force-pushed the feat/child-second-parameter branch from 3bebfbf to a6b44da Compare January 21, 2024 21:30
@Aylur Aylur force-pushed the feat/child-second-parameter branch from 0d35fba to 560e10d Compare January 31, 2024 13:34
@Aylur Aylur merged commit dca6849 into main Jan 31, 2024
2 of 4 checks passed
gorsbart pushed a commit to gorsbart/ags that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant