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

Tailwind-compatible flex section: round 1 #6650

Merged
merged 7 commits into from
Nov 19, 2024

Conversation

bkrmendy
Copy link
Contributor

@bkrmendy bkrmendy commented Nov 18, 2024

Problem

The controls to set justify-contents, align-items and flex-direction don't work with Tailwind

Fix

  • Add the necessary class names to TailwindClassNameMapping, so that the Tailwind style plugin can write these classes. These controls read props from specialSizeMeasurements, so I'm leaving StyleInfo unchanged in this PR, and add the flex-related classes I added to TailwindClassNameMapping when something actually needs to read them through StyleInfo.
  • Add tests for Tailwind editing in the affected controls

Commit Details

  • Add the necessary class names to TailwindClassNameMapping
  • Add tests for NineBlockControl (used to set justify-contents and align-items)
  • Add tests for ThreebarControl (used to set align-items when justify-contents is set to space-between)
  • Add tests for SpacedPackedControls (sets justify-contents to either space-between for spaced or to flex-start for packed)
  • Add tests for FlexDirectionToggle (used to set flex direction)

Out of scope

There are two other controls in the flex section, used for setting flex-wrap and flex-gap. These controls use useInspectorInfo under the hood, making that work is left to a follow-up PR

Manual Tests

I hereby swear that:

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

Copy link
Contributor

github-actions bot commented Nov 18, 2024

Try me

Copy link

relativeci bot commented Nov 18, 2024

#15181 Bundle Size — 58.09MiB (~-0.01%).

8b7cdb1(current) vs d1d46c1 master#15178(baseline)

Warning

Bundle contains 70 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Improvement 1 improvement
                 Current
#15181
     Baseline
#15178
Improvement  Initial JS 41.06MiB(~-0.01%) 41.07MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 18.15% 21.31%
No change  Chunks 20 20
No change  Assets 22 22
No change  Modules 4170 4170
No change  Duplicate Modules 213 213
No change  Duplicate Code 27.29% 27.29%
No change  Packages 477 477
No change  Duplicate Packages 70 70
Bundle size by type  Change 2 changes Improvement 2 improvements
                 Current
#15181
     Baseline
#15178
Improvement  JS 58.08MiB (~-0.01%) 58.08MiB
Improvement  HTML 7.37KiB (-0.25%) 7.39KiB

Bundle analysis reportBranch feature/tailwind-flex-sectionProject dashboard


Generated by RelativeCIDocumentationReport issue

@bkrmendy bkrmendy changed the title flex section with Tailwind Tailwind-compatible flex section: round 1 Nov 18, 2024
@bkrmendy bkrmendy marked this pull request as ready for review November 19, 2024 09:24
Copy link
Contributor

@seanparsons seanparsons left a comment

Choose a reason for hiding this comment

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

All of the tests!

@bkrmendy bkrmendy merged commit bcde12d into master Nov 19, 2024
16 checks passed
@bkrmendy bkrmendy deleted the feature/tailwind-flex-section branch November 19, 2024 13:11
liady pushed a commit that referenced this pull request Dec 13, 2024
## Problem
The controls to set `justify-contents`, `align-items` and
`flex-direction` don't work with Tailwind

## Fix
- Add the necessary class names to `TailwindClassNameMapping`, so that
the Tailwind style plugin can write these classes. These controls read
props from `specialSizeMeasurements`, so I'm leaving `StyleInfo`
unchanged in this PR, and add the flex-related classes I added to
`TailwindClassNameMapping` when something actually needs to read them
through `StyleInfo`.
- Add tests for Tailwind editing in the affected controls

### Commit Details
- Add the necessary class names to `TailwindClassNameMapping`
- Add tests for `NineBlockControl` (used to set `justify-contents` and
`align-items`)
- Add tests for `ThreebarControl` (used to set `align-items` when
`justify-contents` is set to `space-between`)
- Add tests for `SpacedPackedControls` (sets `justify-contents` to
either `space-between` for spaced or to `flex-start` for `packed`)
- Add tests for `FlexDirectionToggle` (used to set flex direction)

### Out of scope
There are two other controls in the flex section, used for setting
`flex-wrap` and `flex-gap`. These controls use `useInspectorInfo` under
the hood, making that work is left to a follow-up PR

## 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants