From 5e3adee8fe0e44dee569a76adbcd135365e19de7 Mon Sep 17 00:00:00 2001 From: Liad Yosef Date: Mon, 16 Dec 2024 16:39:56 +0200 Subject: [PATCH] feat(editor): update correct breakpoint value (#6724) This PR adds the functionality of updating the correct breakpoint value in Tailwind (`lg-`, `md-`, etc) according to the current Scene size. **Details:** - We're checking the current Scene size to see if the element has some variant for this prop that matches it (we're choosing the best one if there are several), and updating only this value - If there are no variants or none matching, we update the default value (no breakpoint) - We're currently updating the value per-breakpoint **only** if it was defined per-breakpoint in the first place (for example via the code editor, with prefix) - The main change is building a map of each property to the modifiers that are *currenty* affecting it - (`getPropertiesToAppliedModifiersMap()`) - so that the property updating methods (that already exist in [tailwind-class-list-utils.ts](https://github.com/concrete-utopia/utopia/compare/feat/update-breakpoint-value?expand=1#diff-a2fc3b378fc847952f33e12c787ded13ff87a7c1f4dc0e3154b73b976117f9f5)) could use this information to understand *which* variant they need to update or remove (instead of always changing the default one as it was until now). Example: (Note - in this PR the controls **are not** indicated in green, this is left for future design decisions) **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 --- .../tailwind-responsive-utils.ts | 40 ++++++++- .../update-class-list.ts | 44 ++++++++-- .../plugins/tailwind-style-plugin.spec.ts | 52 +++++++++++- .../canvas/plugins/tailwind-style-plugin.ts | 1 + .../tailwind-class-list-utils.spec.ts | 24 ++++-- .../tailwind/tailwind-class-list-utils.ts | 81 +++++++++++++++---- 6 files changed, 209 insertions(+), 33 deletions(-) diff --git a/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/tailwind-responsive-utils.ts b/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/tailwind-responsive-utils.ts index 6782d6fee6a0..f77ed1584e51 100644 --- a/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/tailwind-responsive-utils.ts +++ b/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/tailwind-responsive-utils.ts @@ -1,8 +1,11 @@ import type { Config } from 'tailwindcss/types/config' -import { type StyleMediaSizeModifier, type StyleModifier } from '../../canvas-types' +import { isStyleInfoKey, type StyleMediaSizeModifier, type StyleModifier } from '../../canvas-types' import type { ScreenSize } from '../../responsive-types' import { extractScreenSizeFromCss } from '../../responsive-utils' import { mapDropNulls } from '../../../../core/shared/array-utils' +import { getTailwindClassMapping, TailwindPropertyMapping } from '../tailwind-style-plugin' +import { parseTailwindPropertyFactory } from '../tailwind-style-plugin' +import type { StylePluginContext } from '../style-plugins' export const TAILWIND_DEFAULT_SCREENS = { sm: '640px', @@ -73,3 +76,38 @@ export function getModifiers( }) .filter((m): m is StyleMediaSizeModifier => m != null) } + +export function getPropertiesToAppliedModifiersMap( + currentClassNameAttribute: string, + propertyNames: string[], + config: Config | null, + context: StylePluginContext, +): Record { + const parseTailwindProperty = parseTailwindPropertyFactory(config, context) + const classMapping = getTailwindClassMapping(currentClassNameAttribute.split(' '), config) + return propertyNames.reduce((acc, propertyName) => { + if (!isStyleInfoKey(propertyName)) { + return acc + } + const parsedProperty = parseTailwindProperty( + classMapping[TailwindPropertyMapping[propertyName]], + propertyName, + ) + if (parsedProperty?.type == 'property' && parsedProperty.currentVariant.modifiers != null) { + return { + ...acc, + [propertyName]: parsedProperty.currentVariant.modifiers, + } + } else { + return acc + } + }, {} as Record) +} + +export function getTailwindVariantFromAppliedModifier( + appliedModifier: StyleMediaSizeModifier | null, +): string | null { + return appliedModifier?.modifierOrigin?.type === 'tailwind' + ? appliedModifier.modifierOrigin.variant + : null +} diff --git a/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/update-class-list.ts b/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/update-class-list.ts index b8896c93aa64..d1fb8ab9a044 100644 --- a/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/update-class-list.ts +++ b/editor/src/components/canvas/plugins/tailwind-style-plugin-utils/update-class-list.ts @@ -2,7 +2,10 @@ import type { ElementPath } from 'utopia-shared/src/types' import { emptyComments } from 'utopia-shared/src/types' import { mapDropNulls } from '../../../../core/shared/array-utils' import { jsExpressionValue } from '../../../../core/shared/element-template' -import type { PropertiesToUpdate } from '../../../../core/tailwind/tailwind-class-list-utils' +import type { + PropertiesToRemove, + PropertiesToUpdate, +} from '../../../../core/tailwind/tailwind-class-list-utils' import { getParsedClassList, removeClasses, @@ -17,6 +20,8 @@ import type { EditorStateWithPatch } from '../../commands/utils/property-utils' import { applyValuesAtPath } from '../../commands/utils/property-utils' import * as PP from '../../../../core/shared/property-path' import type { Config } from 'tailwindcss/types/config' +import type { StylePluginContext } from '../style-plugins' +import { getPropertiesToAppliedModifiersMap } from './tailwind-responsive-utils' export type ClassListUpdate = | { type: 'add'; property: string; value: string } @@ -37,27 +42,54 @@ export const runUpdateClassList = ( element: ElementPath, classNameUpdates: ClassListUpdate[], config: Config | null, + context: StylePluginContext, ): EditorStateWithPatch => { const currentClassNameAttribute = getClassNameAttribute(getElementFromProjectContents(element, editorState.projectContents)) ?.value ?? '' + // this will map every property to the modifiers that are currently affecting it + const propertyToAppliedModifiersMap = getPropertiesToAppliedModifiersMap( + currentClassNameAttribute, + classNameUpdates.map((update) => update.property), + config, + context, + ) + const parsedClassList = getParsedClassList(currentClassNameAttribute, config) - const propertiesToRemove = mapDropNulls( - (update) => (update.type !== 'remove' ? null : update.property), - classNameUpdates, + const propertiesToRemove: PropertiesToRemove = classNameUpdates.reduce( + (acc: PropertiesToRemove, val) => + val.type === 'remove' + ? { + ...acc, + [val.property]: { + modifiers: propertyToAppliedModifiersMap[val.property] ?? [], + }, + } + : acc, + {}, ) const propertiesToUpdate: PropertiesToUpdate = classNameUpdates.reduce( - (acc: { [property: string]: string }, val) => - val.type === 'remove' ? acc : { ...acc, [val.property]: val.value }, + (acc: PropertiesToUpdate, val) => + val.type === 'remove' + ? acc + : { + ...acc, + [val.property]: { + newValue: val.value, + modifiers: propertyToAppliedModifiersMap[val.property] ?? [], + }, + }, {}, ) const updatedClassList = [ removeClasses(propertiesToRemove), updateExistingClasses(propertiesToUpdate), + // currently we're not adding new breakpoint styles (but only editing current ones), + // so we don't need to pass the propertyToAppliedModifiersMap here addNewClasses(propertiesToUpdate), ].reduce((classList, fn) => fn(classList), parsedClassList) diff --git a/editor/src/components/canvas/plugins/tailwind-style-plugin.spec.ts b/editor/src/components/canvas/plugins/tailwind-style-plugin.spec.ts index cf1e604a3833..aa13df63a9f4 100644 --- a/editor/src/components/canvas/plugins/tailwind-style-plugin.spec.ts +++ b/editor/src/components/canvas/plugins/tailwind-style-plugin.spec.ts @@ -9,6 +9,7 @@ import { TailwindPlugin } from './tailwind-style-plugin' import { createModifiedProject } from '../../../sample-projects/sample-project-utils.test-utils' import { TailwindConfigPath } from '../../../core/tailwind/tailwind-config' import { getTailwindConfigCached } from '../../../core/tailwind/tailwind-compilation' +import * as StylePlugins from './style-plugins' const Project = createModifiedProject({ [StoryboardFilePath]: ` @@ -30,7 +31,7 @@ export var storyboard = ( >
@@ -40,13 +41,16 @@ export var storyboard = ( [TailwindConfigPath]: ` const TailwindConfig = { content: [], - theme: { extend: { gap: { enormous: '222px' } } } + theme: { extend: { gap: { enormous: '222px' }, screens: { lg: '500px' } } } } export default TailwindConfig `, }) describe('tailwind style plugin', () => { + afterAll(() => { + jest.restoreAllMocks() + }) it('can set Tailwind class', async () => { const editor = await renderTestEditorWithModel(Project, 'await-first-dom-report') const target = EP.fromString('sb/scene/div') @@ -64,7 +68,7 @@ describe('tailwind style plugin', () => { updatedEditor.editorStateWithChanges.projectContents, )! expect(formatJSXAttributes(normalizedElement.props)).toEqual({ - className: 'flex flex-col gap-[222px]', + className: 'flex flex-col gap-[222px] pt-[10px] lg:pt-[20px]', 'data-uid': 'div', }) }) @@ -84,7 +88,47 @@ describe('tailwind style plugin', () => { updatedEditor.editorStateWithChanges.projectContents, )! expect(formatJSXAttributes(normalizedElement.props)).toEqual({ - className: 'flex flex-col gap-enormous', + className: 'flex flex-col gap-enormous pt-[10px] lg:pt-[20px]', + 'data-uid': 'div', + }) + }) + + it('can set Tailwind class with size modifier and a custom config', async () => { + // this is done since we don't calculate the scene size in the test + jest.spyOn(StylePlugins, 'sceneSize').mockReturnValue({ type: 'scene', width: 700 }) + const editor = await renderTestEditorWithModel(Project, 'await-first-dom-report') + const target = EP.fromString('sb/scene/div') + const updatedEditor = TailwindPlugin( + getTailwindConfigCached(editor.getEditorState().editor), + ).updateStyles(editor.getEditorState().editor, target, [ + { type: 'set', property: 'paddingTop', value: '200px' }, + ]) + const normalizedElement = getJSXElementFromProjectContents( + target, + updatedEditor.editorStateWithChanges.projectContents, + )! + expect(formatJSXAttributes(normalizedElement.props)).toEqual({ + className: 'flex flex-row gap-[12px] pt-[10px] lg:pt-[200px]', + 'data-uid': 'div', + }) + }) + + it('can remove Tailwind class with size modifier and a custom config', async () => { + // this is done since we don't calculate the scene size in the test + jest.spyOn(StylePlugins, 'sceneSize').mockReturnValue({ type: 'scene', width: 700 }) + const editor = await renderTestEditorWithModel(Project, 'await-first-dom-report') + const target = EP.fromString('sb/scene/div') + const updatedEditor = TailwindPlugin( + getTailwindConfigCached(editor.getEditorState().editor), + ).updateStyles(editor.getEditorState().editor, target, [ + { type: 'delete', property: 'paddingTop' }, + ]) + const normalizedElement = getJSXElementFromProjectContents( + target, + updatedEditor.editorStateWithChanges.projectContents, + )! + expect(formatJSXAttributes(normalizedElement.props)).toEqual({ + className: 'flex flex-row gap-[12px] pt-[10px]', 'data-uid': 'div', }) }) diff --git a/editor/src/components/canvas/plugins/tailwind-style-plugin.ts b/editor/src/components/canvas/plugins/tailwind-style-plugin.ts index 5f378167a27c..2218753fdb26 100644 --- a/editor/src/components/canvas/plugins/tailwind-style-plugin.ts +++ b/editor/src/components/canvas/plugins/tailwind-style-plugin.ts @@ -279,6 +279,7 @@ export const TailwindPlugin = (config: Config | null): StylePlugin => ({ elementPath, [...propsToDelete, ...propsToSet], config, + { sceneSize: getContainingSceneSize(elementPath, editorState.jsxMetadata) }, ) }, }) diff --git a/editor/src/core/tailwind/tailwind-class-list-utils.spec.ts b/editor/src/core/tailwind/tailwind-class-list-utils.spec.ts index ca8816febed2..6b1594367774 100644 --- a/editor/src/core/tailwind/tailwind-class-list-utils.spec.ts +++ b/editor/src/core/tailwind/tailwind-class-list-utils.spec.ts @@ -141,7 +141,10 @@ describe('tailwind class list utils', () => { describe('removing classes', () => { it('can remove property', () => { const classList = getParsedClassList('p-4 m-2 text-white w-4 flex flex-row', null) - const updatedClassList = removeClasses(['padding', 'textColor'])(classList) + const updatedClassList = removeClasses({ + padding: { modifiers: [] }, + textColor: { modifiers: [] }, + })(classList) expect(getClassListFromParsedClassList(updatedClassList, null)).toMatchInlineSnapshot( `"m-2 w-4 flex flex-row"`, ) @@ -151,7 +154,10 @@ describe('tailwind class list utils', () => { 'p-4 m-2 text-white hover:text-red-100 w-4 flex flex-row', null, ) - const updatedClassList = removeClasses(['padding', 'textColor'])(classList) + const updatedClassList = removeClasses({ + padding: { modifiers: [] }, + textColor: { modifiers: [] }, + })(classList) expect(getClassListFromParsedClassList(updatedClassList, null)).toMatchInlineSnapshot( `"m-2 hover:text-red-100 w-4 flex flex-row"`, ) @@ -162,8 +168,8 @@ describe('tailwind class list utils', () => { it('can update class in class list', () => { const classList = getParsedClassList('p-4 m-2 text-white w-4 flex flex-row', null) const updatedClassList = updateExistingClasses({ - flexDirection: 'column', - width: '23px', + flexDirection: { newValue: 'column', modifiers: [] }, + width: { newValue: '23px', modifiers: [] }, })(classList) expect(getClassListFromParsedClassList(updatedClassList, null)).toMatchInlineSnapshot( `"p-4 m-2 text-white w-[23px] flex flex-col"`, @@ -171,7 +177,9 @@ describe('tailwind class list utils', () => { }) it('does not remove property with selector', () => { const classList = getParsedClassList('p-4 hover:p-6 m-2 text-white w-4 flex flex-row', null) - const updatedClassList = updateExistingClasses({ padding: '8rem' })(classList) + const updatedClassList = updateExistingClasses({ + padding: { newValue: '8rem', modifiers: [] }, + })(classList) expect(getClassListFromParsedClassList(updatedClassList, null)).toMatchInlineSnapshot( `"p-32 hover:p-6 m-2 text-white w-4 flex flex-row"`, ) @@ -182,9 +190,9 @@ describe('tailwind class list utils', () => { it('can add new class to class list', () => { const classList = getParsedClassList('p-4 m-2 text-white w-4 flex flex-row', null) const updatedClassList = addNewClasses({ - backgroundColor: 'white', - justifyContent: 'space-between', - positionLeft: '-20px', + backgroundColor: { newValue: 'white', modifiers: [] }, + justifyContent: { newValue: 'space-between', modifiers: [] }, + positionLeft: { newValue: '-20px', modifiers: [] }, })(classList) expect(getClassListFromParsedClassList(updatedClassList, null)).toMatchInlineSnapshot( `"p-4 m-2 text-white w-4 flex flex-row bg-white justify-between -left-[20px]"`, diff --git a/editor/src/core/tailwind/tailwind-class-list-utils.ts b/editor/src/core/tailwind/tailwind-class-list-utils.ts index fd6152c79f32..dbd30bdf307e 100644 --- a/editor/src/core/tailwind/tailwind-class-list-utils.ts +++ b/editor/src/core/tailwind/tailwind-class-list-utils.ts @@ -1,11 +1,12 @@ import * as TailwindClassParser from '@xengine/tailwindcss-class-parser' import type { Config } from 'tailwindcss/types/config' import { mapDropNulls } from '../shared/array-utils' +import type { StyleMediaSizeModifier, StyleModifier } from '../../components/canvas/canvas-types' export type ParsedTailwindClass = { property: string value: string - variants: unknown[] + variants: { type: string; value: string }[] negative: boolean } & Record @@ -49,7 +50,13 @@ export type ClassListTransform = ( ) => TailwindClassParserResult[] export interface PropertiesToUpdate { - [property: string]: string + [property: string]: { newValue: string; modifiers: StyleModifier[] } +} + +export interface PropertiesToRemove { + [property: string]: { + modifiers: StyleModifier[] + } } export const addNewClasses = @@ -60,12 +67,12 @@ export const addNewClasses = ) const newClasses: TailwindClassParserResult[] = mapDropNulls( - ([prop, value]) => + ([prop, update]) => existingProperties.has(prop) ? null : { type: 'parsed', - ast: { property: prop, value: value, variants: [], negative: false }, + ast: { property: prop, value: update.newValue, variants: [], negative: false }, }, Object.entries(propertiesToAdd), ) @@ -78,30 +85,76 @@ export const updateExistingClasses = (propertiesToUpdate: PropertiesToUpdate): ClassListTransform => (parsedClassList: TailwindClassParserResult[]) => { const classListWithUpdatedClasses: TailwindClassParserResult[] = parsedClassList.map((cls) => { - if (cls.type !== 'parsed' || cls.ast.variants.length > 0) { - return cls - } - const updatedProperty = propertiesToUpdate[cls.ast.property] - if (updatedProperty == null) { + if (!shouldUpdateClass(cls, propertiesToUpdate)) { return cls } + const propertyToUpdate = propertiesToUpdate[cls.ast.property] return { type: 'parsed', - ast: { property: cls.ast.property, value: updatedProperty, variants: [], negative: false }, + ast: { + property: cls.ast.property, + value: propertyToUpdate.newValue, + variants: cls.ast.variants, + negative: false, + }, } }) return classListWithUpdatedClasses } export const removeClasses = - (propertiesToRemove: string[]): ClassListTransform => + (propertiesToRemove: PropertiesToRemove): ClassListTransform => (parsedClassList: TailwindClassParserResult[]) => { - const propertiesToRemoveSet = new Set(propertiesToRemove) const classListWithRemovedClasses = parsedClassList.filter((cls) => { - if (cls.type !== 'parsed' || cls.ast.variants.length > 0) { + if (!shouldUpdateClass(cls, propertiesToRemove)) { return cls } - return !propertiesToRemoveSet.has(cls.ast.property) + return propertiesToRemove[cls.ast.property] == null }) return classListWithRemovedClasses } + +function getTailwindSizeVariant(modifiers: StyleModifier[]): string | null { + const mediaModifier = modifiers.find((m): m is StyleMediaSizeModifier => m.type === 'media-size') + if (mediaModifier == null) { + return null + } + if (mediaModifier.modifierOrigin?.type !== 'tailwind') { + return null + } + return mediaModifier.modifierOrigin.variant +} + +function shouldUpdateClass( + cls: TailwindClassParserResult, + propertiesToUpdate: PropertiesToUpdate | PropertiesToRemove, +): cls is TailwindClassParserResult & { type: 'parsed' } { + if (cls.type !== 'parsed') { + return false + } + const propertyToUpdate = propertiesToUpdate[cls.ast.property] + if (propertyToUpdate == null) { + // this property is not in the list + return false + } + const sizeVariantToUpdate = getTailwindSizeVariant(propertyToUpdate.modifiers) + if (sizeVariantToUpdate == null && cls.ast.variants.length > 0) { + // we need to update the default property value but this class has variants + return false + } + if ( + sizeVariantToUpdate != null && + !variantsHasMediaSizeVariant(cls.ast.variants, sizeVariantToUpdate) + ) { + // we need to update a specific size variant but this class doesn't have it + return false + } + return true +} + +function variantsHasMediaSizeVariant( + variants: { type: string; value: string }[], + sizeVariant: string, +): boolean { + return variants.some((v) => v.type === 'media' && v.value === sizeVariant) +}