GridMeasurementHelper component to continuously measure grid cells #6517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#14793 Bundle Size — 57.96MiB (~+0.01%).360a854(current) vs 916e8b9 master#14786(baseline) Warning Bundle contains 70 duplicate packages – View duplicate packages Bundle metrics
|
| Current #14793 |
Baseline #14786 |
|
|---|---|---|
 Initial JS |
40.95MiB(~+0.01%) |
40.94MiB |
 Initial CSS |
0B |
0B |
 Cache Invalidation |
17.97% |
17.85% |
| Chunks |
20 |
20 |
| Assets |
22 |
22 |
| Modules |
4148 |
4148 |
| Duplicate Modules |
213 |
213 |
| Duplicate Code |
27.34% |
27.34% |
| Packages |
477 |
477 |
| Duplicate Packages |
70 |
70 |
Bundle size by type 2 changes 1 regression 
1 improvement
| Current #14793 |
Baseline #14786 |
|
|---|---|---|
| JS |
57.95MiB (~+0.01%) |
57.95MiB |
| HTML |
7.37KiB (-0.25%) |
7.39KiB |
Bundle analysis report Branch fix/reparent-blinking Project dashboard
Generated by RelativeCI Documentation Report issue
gbalint
requested review from
seanparsons,
Rheeseyb,
ruggi,
balazsbajorics,
liady and
bkrmendy
ruggi
approved these changes
Oct 11, 2024
gbalint
changed the title
Try me(building...) |
Try me(deploying...) |
ruggi
approved these changes
Oct 14, 2024
bkrmendy
approved these changes
Oct 15, 2024
liady
pushed a commit
that referenced
this pull request
Dec 13, 2024
…6517) **Problem:** When you reparent from a grid to another grid, for a single frame the element blinks at the same cell position as it was in the original grid: https://screenshot.click/09-31-10wrd-ysw94.mp4 **Root cause:** As you hover over the target grid, the grid-reparent-strategy is selected. It triggers a showGridControl command, so the grid control is shown and the cells can be measured. However, it won't be able to position to the real target cell in this frame yet, because the control is not enabled yet, so we don't have the cell bounds in the metadata. **Fix:** Instead of using grid controls to measure let show permanent grid measurement helpers so grids can always be measured. **Commit Details:** (< vv pls delete this section if's not relevant) - Added memoized function `getAllGrids` which returns all the grids from the metadata - Added `GridMeasurementHelper` component which renders a grid with placeholder divs in its cells (similarly to `GridControl`, but much-much simpler) - Rendering `GridMeasurementHelper` for all grids - Use rendered grid measurement helpers in dom-walker to measure the size of grid cell bounds - Remove all the hacks (most notably `getMetadataWithGridCellBounds`) which made it possible to use latestMetadata in interaction when startingMetadata did not include grid cell bounds. This is not needed anymore, because the grid cells are always measured using the always-on `GridMeasurementHelper` components **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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
When you reparent from a grid to another grid, for a single frame the element blinks at the same cell position as it was in the original grid: https://screenshot.click/09-31-10wrd-ysw94.mp4
Root cause:
As you hover over the target grid, the grid-reparent-strategy is selected. It triggers a showGridControl command, so the grid control is shown and the cells can be measured. However, it won't be able to position to the real target cell in this frame yet, because the control is not enabled yet, so we don't have the cell bounds in the metadata.
Fix:
Instead of using grid controls to measure let show permanent grid measurement helpers so grids can always be measured.
Commit Details: (< vv pls delete this section if's not relevant)
getAllGridswhich returns all the grids from the metadataGridMeasurementHelpercomponent which renders a grid with placeholder divs in its cells (similarly toGridControl, but much-much simpler)GridMeasurementHelperfor all gridsgetMetadataWithGridCellBounds) which made it possible to use latestMetadata in interaction when startingMetadata did not include grid cell bounds. This is not needed anymore, because the grid cells are always measured using the always-onGridMeasurementHelpercomponentsManual Tests:
I hereby swear that: