Add `gridLayoutOptions` as optional parameter for `useGridLayout` hook

[Nilomiranda]

Situation

The useGridLayout hook is very handy and provides grid configurations based on the grid container width. However it's limited to a fixed set of grid configurations and if a screen is larger it will always fallback to the 5x5 grid which is the largest available today

Proposal

This PR adds an optional option.gridLayouts parameters so when using the hook, we can optionally pass our own custom grid configurations, either by extending the existing one (already being exported here) or writing your own set.

Second "optional" proposal

Return width and height from the hook as it could be useful to have access to the width and height determined by the logic inside the hook to run, if needed, some additional logic within the component using the useGridLayout hook. I'm marking this as optional because if the team thinks this shouldn't exist, I can happily exclude it.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 839da683fca26a0657a78b068fd453fa78a1f2e9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[CLAassistant]

All committers have signed the CLA.

[Ocupe]

Hey @Nilomiranda , thanks for taking the time to contribute. I think giving the user the option to pass in layouts makes sense and we should add it. Before we approve this, we should make sure that we remove all sharp edges and possible footguns to make the usage as simple and solid as possible.

I see some possible simplifications, and some footguns that we should remove:

• Not all values of the current GridLayoutDefinition are needed. Maybe we can get rid of the maxTiles (since we can calculate it from the columns and rows) and name values.
• The selectGridLayout function expects the layouts to be sorted from small to large. I wonder how we could make sure that the developers are aware of this or whether we should sort them by default. 🤔

[lukasIO]

I would also vote for a simplified approach. We currently use auto-rows in CSS anyways, so the rows value doesn't really do anything. Given that, I think columns + maxTiles would suffice ?

[Nilomiranda]

Hey @Nilomiranda , thanks for taking the time to contribute. I think giving the user the option to pass in layouts makes sense and we should add it. Before we approve this, we should make sure that we remove all sharp edges and possible footguns to make the usage as simple and solid as possible.

I see some possible simplifications, and some footguns that we should remove:

• Not all values of the current GridLayoutDefinition are needed. Maybe we can get rid of the maxTiles (since we can calculate it from the columns and rows) and name values.
• The selectGridLayout function expects the layouts to be sorted from small to large. I wonder how we could make sure that the developers are aware of this or whether we should sort them by default. 🤔

Hi @Ocupe

Unfortunately, maxTiles is exactly the property I need to be able to set the most along with the number of rows and columns. Actually I don't see any of the properties of the layout definition being "useless" and can depend on the case.

And to expand on what @lukasIO said here:

I would also vote for a simplified approach. We currently use auto-rows in CSS anyways, so the rows value doesn't really do anything.

I would agree but there may be cases where I want to build the grid component myself (it is the case with me btw) so I use whatever is defined in rows and columns to determine the number of grid rows and grid columns my grid element will need.

The selectGridLayout function expects the layouts to be sorted from small to large. I wonder how we could make sure that the developers are aware of this or whether we should sort them by default. 🤔

We could either add a comment that would serve as documentation or sort ourselves. How do you determine what is larger? By the product of col * rows? By the maxTiles or minTiles?

[Ocupe]

Unfortunately, maxTiles is exactly the property I need to be able to set the most along with the number of rows and columns. Actually I don't see any of the properties of the layout definition being "useless" and can depend on the case.

I didn't communicate that well. I meant we don't need to force the user to provide these fields because we can calculate them from other fields provided by the user. We can compute them from the input and still use them in the function. Example:

// Layout definition provided by the user:
{
    columns: 2,
    rows: 2,
    minTiles: 3,
    minWidth: 560,
    minHeight: 0,
}

 // Layout definition derived from the provided layout definition:
{
    columns: 2,
    rows: 2,
    name: '2x2', // computed from columns & rows
    minTiles: 3,
    maxTiles: 4, // computed from columns & rows
    minWidth: 560,
    minHeight: 0,
}

We may be able to get away with making minWidth and minHeight optional and assuming 0 if no value is provided. 🤔

I would also vote for a simplified approach. We currently use auto-rows in CSS anyways, so the rows value doesn't really do anything.

I would agree but there may be cases where I want to build the grid component myself (it is the case with me btw) so I use whatever is defined in rows and columns to determine the number of grid rows and grid columns my grid element will need.

I think defining a gird by rows and columns feels very familiar and explicit.

The selectGridLayout function expects the layouts to be sorted from small to large. I wonder how we could make sure that the developers are aware of this or whether we should sort them by default. 🤔

We could either add a comment that would serve as documentation or sort ourselves. How do you determine what is larger? By the product of col * rows? By the maxTiles or minTiles?

I think the sort should be by maxTiles first and if two layouts have the same maxTiles I would sort them by their minWidth / minHeight values.

[Ocupe]

@Nilomiranda, @lukasIO check out my proposal here: https://github.com/livekit/components-js/pull/909

[Nilomiranda]

@Nilomiranda, @lukasIO check out my proposal here: #909

Just left my review on your proposal, thanks!

[lukasIO]

@Ocupe what does the auto row behaviour in CSS do in this case? It doesn't look like you're removing that in #909?

[Nilomiranda]

@Ocupe @lukasIO I believe we can close this PR right? I saw that the extended version #909 was merged.