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

Gap controls centered relatively to content #4334

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

ruggi
Copy link
Contributor

@ruggi ruggi commented Oct 6, 2023

Fixes #4333

Problem:

Flex gap handles are centered relatively to the container size. This means that they can become hard to find if the container.

E.g.

Screenshot 2023-10-06 at 11 46 15 AM

Fix:

  1. Place the control handles centered relatively to the content bounds, not the container bounds
  2. Make the control handles brandNeonPink to make them easier to spot
Screenshot 2023-10-06 at 11 47 18 AM

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Try me

@relativeci
Copy link

relativeci bot commented Oct 6, 2023

Job #8475: Bundle Size — 62.83MiB (~+0.01%).

83488bb(current) vs 5957b21 master#8464(baseline)

Warning

Bundle contains 64 duplicate packages - View duplicate packages

Bundle metrics (1 change)
                 Current
Job #8475
     Baseline
Job #8464
Initial JS 35.1MiB(~+0.01%) 35.1MiB
Initial CSS 0B 0B
Cache Invalidation 19.4% 19.4%
Chunks 27 27
Assets 31 31
Modules 3974 3974
Duplicate Modules 425 425
Duplicate Code 30.92% 30.92%
Packages 409 409
Duplicate Packages 64 64
Bundle size by type (1 change)
                 Current
Job #8475
     Baseline
Job #8464
CSS 0B 0B
Fonts 0B 0B
HTML 11.43KiB 11.43KiB
IMG 0B 0B
JS 62.82MiB (~+0.01%) 62.82MiB
Media 0B 0B
Other 0B 0B

View job #8475 reportView feat/gap-control-content-area branch activity

@@ -125,7 +125,10 @@ export const setFlexGapStrategy: CanvasStrategyFactory = (
resizeControl,
controlWithProps({
control: FloatingIndicator,
props: { ...props, color: colorTheme.gapControls.value },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that gapControls should be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmmh it's still there for the striped background, maybe worth renaming it to gapControlsBg or something like that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds pretty sensible to me at least.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Performance test results:
(Chart1)
(Chart2)

@ruggi ruggi merged commit 0780241 into master Oct 6, 2023
@ruggi ruggi deleted the feat/gap-control-content-area branch October 6, 2023 12:43
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.

Place flex gap handles relative to the contents
3 participants