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

Style plugins #6513

Merged
merged 10 commits into from
Oct 10, 2024
Merged

Style plugins #6513

merged 10 commits into from
Oct 10, 2024

Conversation

bkrmendy
Copy link
Contributor

@bkrmendy bkrmendy commented Oct 10, 2024

Example project with a flex gap

Problem

The canvas controls can't edit elements that are styled with Tailwind

Fix

Add a plugin system, where each plugin provides a data source that strategies can read from (so as of this PR, either Tailwind or the inline style), and a function that takes props from the inline style, and moves them to the right place the style element. For example, the plugin that implements Tailwind styling takes the inline style props, converts them to the right tailwind classes, and adds them to the className prop.

The flex gap strategy and the flex gap control is updated to use the data provided by the plugin system to read the necessary data.

Out of scope

There's one known limitation of this system as implemented in this PR: props that are removed by a strategy don't get removed by the normalization step in plugins. This is only a problem for the Tailwind plugin (since the inline style plugin leaves the inline style prop as the strategies left them, so the fix for this will come on a follow-up PR after this one (the actual fix is already implemented here for anyone interested b2fc141)

Details

  • Types for the plugin system, and two concrete plugin implementations are added in the canvas/plugins folder
  • The data provided by the plugins is added to InteractionCanvasState as the styleInfoReader function
  • The normalization step provided by the plugins is hooked into the strategy lifecycle in interactionFinished
  • The flex gap strategy/controls are updated to use styleInfoReader
  • UpdateClassList is fixed so that it can write the className prop even if it doesn't exist on element at the start of the command

Copy link
Contributor

github-actions bot commented Oct 10, 2024

Try me

Copy link

relativeci bot commented Oct 10, 2024

#14699 Bundle Size — 57.94MiB (+0.01%).

0c27924(current) vs 916e8b9 master#14690(baseline)

Warning

Bundle contains 70 duplicate packages – View duplicate packages

Bundle metrics  Change 3 changes Regression 1 regression
                 Current
#14699
     Baseline
#14690
Regression  Initial JS 40.94MiB(+0.02%) 40.93MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 17.93% 17.91%
No change  Chunks 21 21
No change  Assets 23 23
Change  Modules 4145(+0.07%) 4142
No change  Duplicate Modules 213 213
No change  Duplicate Code 27.32% 27.32%
No change  Packages 477 477
No change  Duplicate Packages 70 70
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
#14699
     Baseline
#14690
Regression  JS 57.93MiB (+0.01%) 57.92MiB
Improvement  HTML 7.37KiB (-0.25%) 7.39KiB

Bundle analysis reportBranch feature/style-pluginsProject dashboard


Generated by RelativeCIDocumentationReport issue


const flexGapValue = updatedGapValue ?? flexGap.value
const flexGapFromEditor = useEditorState(
Copy link
Contributor Author

@bkrmendy bkrmendy Oct 10, 2024

Choose a reason for hiding this comment

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

the refactor here was necessary because the existing early return didn't respect the rules of hooks, and broke some tests after the flex gap was calculated from the data provided by the plugins

@bkrmendy bkrmendy marked this pull request as ready for review October 10, 2024 10:47
@bkrmendy bkrmendy changed the title Feature/style plugins Style plugins Oct 10, 2024
Copy link
Contributor

@ruggi ruggi left a comment

Choose a reason for hiding this comment

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

note for future selves: gap is not only for flex, but also for grids (https://developer.mozilla.org/en-US/docs/Web/CSS/gap), so maybe we should take that into account when naming things (e.g. flexGap)

bkrmendy and others added 2 commits October 10, 2024 15:08
Co-authored-by: Federico Ruggi <1081051+ruggi@users.noreply.github.com>
…trol.tsx

Co-authored-by: Federico Ruggi <1081051+ruggi@users.noreply.github.com>
@bkrmendy bkrmendy merged commit 647fb23 into master Oct 10, 2024
13 checks passed
@bkrmendy bkrmendy deleted the feature/style-plugins branch October 10, 2024 14:00
liady pushed a commit that referenced this pull request Dec 13, 2024
# [Example project with a flex
gap](https://utopia.fish/p/71b4ada2-boatneck-lamp/?branch_name=feature-style-plugins)

## Problem
The canvas controls can't edit elements that are styled with Tailwind

## Fix
Add a plugin system, where each plugin provides a data source that
strategies can read from (so as of this PR, either Tailwind or the
inline style), and a function that takes props from the inline style,
and moves them to the right place the style element. For example, the
plugin that implements Tailwind styling takes the inline style props,
converts them to the right tailwind classes, and adds them to the
`className` prop.

The flex gap strategy and the flex gap control is updated to use the
data provided by the plugin system to read the necessary data.

### Out of scope
There's one known limitation of this system as implemented in this PR:
props that are removed by a strategy don't get removed by the
normalization step in plugins. This is only a problem for the Tailwind
plugin (since the inline style plugin leaves the inline style prop as
the strategies left them, so the fix for this will come on a follow-up
PR after this one (the actual fix is already implemented here for anyone
interested
b2fc141)

### Details
- Types for the plugin system, and two concrete plugin implementations
are added in the `canvas/plugins` folder
- The data provided by the plugins is added to `InteractionCanvasState`
as the `styleInfoReader` function
- The normalization step provided by the plugins is hooked into the
strategy lifecycle in `interactionFinished`
- The flex gap strategy/controls are updated to use `styleInfoReader`
- `UpdateClassList` is fixed so that it can write the `className` prop
even if it doesn't exist on element at the start of the command

---------

Co-authored-by: Federico Ruggi <1081051+ruggi@users.noreply.github.com>
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.

4 participants