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

Correctly position grid resize controls. #6044

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

seanparsons
Copy link
Contributor

@seanparsons seanparsons commented Jul 8, 2024

Problem:
The grid resize controls had their positioning quickly thrown together and somewhat implemented by eye.

Fix:
As part of this work the two big blocks of code for columns and rows were unified into a single GridResizing component.

The positions have been better positioned by creating a container element which straddles the center line of the gap and then the control is appropriately justified within that to keep it centered or in the case of the row controls right justified.

It also uses the CanvasLabel component to handle most of the work for actually displaying things.

Screenshot:
image

Commit Details:

  • Create a common component of GridResizing.
  • Use the new GridResizing component in place of the embedded column and row handling.

Manual Tests:
I hereby swear that:

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

- Create a common component of `GridResizing`.
- Use the new `GridResizing` component in place of the embedded column and row
  handling.
Copy link
Contributor

github-actions bot commented Jul 8, 2024

Try me

Copy link

relativeci bot commented Jul 8, 2024

#13277 Bundle Size — 62.5MiB (~-0.01%).

e588b3c(current) vs 6339ece master#13264(baseline)

Warning

Bundle contains 51 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Improvement 1 improvement
                 Current
#13277
     Baseline
#13264
Improvement  Initial JS 45.56MiB(~-0.01%) 45.56MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 21.62% 21.84%
No change  Chunks 30 30
No change  Assets 33 33
No change  Modules 4323 4323
No change  Duplicate Modules 524 524
No change  Duplicate Code 31.74% 31.74%
No change  Packages 450 450
No change  Duplicate Packages 51 51
Bundle size by type  Change 2 changes Improvement 2 improvements
                 Current
#13277
     Baseline
#13264
Improvement  JS 62.49MiB (~-0.01%) 62.5MiB
Improvement  HTML 11.16KiB (-0.33%) 11.2KiB

Bundle analysis reportBranch fix/grid-resize-controlsProject dashboard

@seanparsons seanparsons merged commit 8b7dd1d into master Jul 9, 2024
15 checks passed
@seanparsons seanparsons deleted the fix/grid-resize-controls branch July 9, 2024 10:09
liady pushed a commit that referenced this pull request Dec 13, 2024
- Create a common component of `GridResizing`.
- Use the new `GridResizing` component in place of the embedded column
and row handling.
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.

3 participants