Grid component support preparation #6636

gbalint · 2024-11-13T15:53:16Z

Problem:
This PR is a preparation step to support strategies for grid items in the following scenario:

the grid is in a component
the grid renders the component's children
the grid is not the root element of the component

The main issue is that we still identify the grid control using the element path of the grid, even though that grid doesn't have a path in this case. We need to be able to show and manage a grid control using the element path of one of its items.

Fix:
Instead of storing the element path of the GridControlData, I introduced a new type GridIdentifier which can identify a grid either by its container element path, or an item's element path.

I updated the strategies and commands to use this grid identifier, and also made sure the strategies for grid items identify using the item, and do not refer to the parent path as the grid. (For reparent we can not really do that, because the item is not in the grid yet. Solving that is not in scope now).

What is missing:
I still use mostly the old control implementations, it only works now because the useGridData hook converts GridItemIdentifier-s to a GridContainerIdentifier-s (using parentPath). So this PR is just a preparation, it does not solve the component problem yet. Next step is to make sure the control can render on the parent element when it receives a GridItemIdentifier.

Manual Tests:
I hereby swear that:

I opened a hydrogen project and it loaded
I could navigate to various routes in Play mode

github-actions · 2024-11-13T15:56:16Z

Try me

relativeci · 2024-11-13T15:59:16Z

#15125 Bundle Size — 58.08MiB (~+0.01%).

b791ff5(current) vs d1d46c1 master#15114(baseline)

Warning

Bundle contains 70 duplicate packages – View duplicate packages

Bundle metrics

2 changes

1 regression

	Current #15125	Baseline #15114
Initial JS	`41.05MiB`(`~+0.01%`)	`41.05MiB`
Initial CSS	`0B`	`0B`
Cache Invalidation	`18.13%`	`18.01%`
Chunks	`20`	`20`
Assets	`22`	`22`
Modules	`4169`	`4169`
Duplicate Modules	`213`	`213`
Duplicate Code	`27.3%`	`27.3%`
Packages	`477`	`477`
Duplicate Packages	`70`	`70`

Bundle size by type

2 changes

1 regression

1 improvement

	Current #15125	Baseline #15114
JS	`58.07MiB` (`~+0.01%`)	`58.07MiB`
HTML	`7.37KiB` (`-0.25%`)	`7.39KiB`

Bundle analysis report Branch chore/grid-component-support Project dashboard

^{Generated by RelativeCI Documentation Report issue}

**Problem:** Followup to #6636 The goal is to support interaction with grid item in the following scenario: - the grid is in a component - the grid renders the component's children - the grid is not the root element of the component **Fix:** In #6636 I introduced a way to indentify a grid using the element path of its item (which is necessary because when the grid is in a non-focused component). - `GridMeasurementHelper` collects the grids from the metadata. These grids in non-focused components are not in the metadata, so they are not collected. I fixed this by collecting component items from the grids too. - To get the measurement data for these grids which identified by their items, I added a new parameter to `getGridMeasurementHelperData` so it can get the styling of the parentElement of the item's element. - The measurement helper was identified by its element path, and the dom-sampler could retrieve the helper from the dom using that. However, this is again not a good solution for these grids in non-focused components, so I added global WeakMap which stores the pairing between HTMLElements of grids and the id attribute of their GridMeasurementHelper. - I updated the `path` fiels names of `GridContainerIdentifier` and `GridItemIdentifier`: it was not a good idea to use the same name in the two paths, because it was possible to not check the type and use the path field. **Missing:** `gridMoveStrategiesExtraCommands` still tries to set grid props on the parent, and when it is a component, those can be absolutely useless. We should detect when that is the case, and not generate these commands. **Manual Tests:** I hereby swear that: - [x] I opened a hydrogen project and it loaded - [x] I could navigate to various routes in Play mode

**Problem:** This PR is a preparation step to support strategies for grid items in the following scenario: - the grid is in a component - the grid renders the component's children - the grid is not the root element of the component The main issue is that we still identify the grid control using the element path of the grid, even though that grid doesn't have a path in this case. We need to be able to show and manage a grid control using the element path of one of its items. **Fix:** Instead of storing the element path of the `GridControlData`, I introduced a new type `GridIdentifier` which can identify a grid either by its container element path, or an item's element path. I updated the strategies and commands to use this grid identifier, and also made sure the strategies for grid items identify using the item, and do not refer to the parent path as the grid. (For reparent we can not really do that, because the item is not in the grid yet. Solving that is not in scope now). What is missing: I still use mostly the old control implementations, it only works now because the `useGridData` hook converts GridItemIdentifier-s to a GridContainerIdentifier-s (using parentPath). So this PR is just a preparation, it does not solve the component problem yet. Next step is to make sure the control can render on the parent element when it receives a GridItemIdentifier. **Manual Tests:** I hereby swear that: - [x] I opened a hydrogen project and it loaded - [x] I could navigate to various routes in Play mode

**Problem:** Followup to #6636 The goal is to support interaction with grid item in the following scenario: - the grid is in a component - the grid renders the component's children - the grid is not the root element of the component **Fix:** In #6636 I introduced a way to indentify a grid using the element path of its item (which is necessary because when the grid is in a non-focused component). - `GridMeasurementHelper` collects the grids from the metadata. These grids in non-focused components are not in the metadata, so they are not collected. I fixed this by collecting component items from the grids too. - To get the measurement data for these grids which identified by their items, I added a new parameter to `getGridMeasurementHelperData` so it can get the styling of the parentElement of the item's element. - The measurement helper was identified by its element path, and the dom-sampler could retrieve the helper from the dom using that. However, this is again not a good solution for these grids in non-focused components, so I added global WeakMap which stores the pairing between HTMLElements of grids and the id attribute of their GridMeasurementHelper. - I updated the `path` fiels names of `GridContainerIdentifier` and `GridItemIdentifier`: it was not a good idea to use the same name in the two paths, because it was possible to not check the type and use the path field. **Missing:** `gridMoveStrategiesExtraCommands` still tries to set grid props on the parent, and when it is a component, those can be absolutely useless. We should detect when that is the case, and not generate these commands. **Manual Tests:** I hereby swear that: - [x] I opened a hydrogen project and it loaded - [x] I could navigate to various routes in Play mode

first commit

a7b509d

gbalint added 4 commits November 13, 2024 17:02

Add comment

fee1e57

Fix GridResizeControlsComponent

f2d37ce

Fix reorder after reparent

9a32681

Fix build error

4f3c191

gbalint marked this pull request as ready for review November 13, 2024 18:09

gbalint changed the title ~~Grid component support~~ Grid component support preparation Nov 13, 2024

ruggi approved these changes Nov 13, 2024

View reviewed changes

Extract gridIdentifierSimilar

b791ff5

bkrmendy approved these changes Nov 14, 2024

View reviewed changes

gbalint merged commit 72d682b into master Nov 14, 2024
14 checks passed

gbalint deleted the chore/grid-component-support branch November 14, 2024 09:16

This was referenced Nov 15, 2024

Grid component support #6640

Merged

Only show grid lines when selecting grid or during child interaction #6647

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grid component support preparation #6636

Grid component support preparation #6636

gbalint commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

relativeci bot commented Nov 13, 2024 •

edited

Loading

Grid component support preparation #6636

Grid component support preparation #6636

Conversation

gbalint commented Nov 13, 2024 • edited Loading

github-actions bot commented Nov 13, 2024 • edited Loading

Try me

relativeci bot commented Nov 13, 2024 • edited Loading

#15125 Bundle Size — 58.08MiB (~+0.01%).

gbalint commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

relativeci bot commented Nov 13, 2024 •

edited

Loading