[fix] style changes (#1814)

This PR updates the way that styles are changed. It splits `setStyle`
and `setOpacity` into `setStyleForNext Shape` and
`setOpacityForNextShape` and `setStyleForSelectedShapes` and
`setOpacityForSelectedShapes`. It fixes the issue with setting one style
re-setting other styles.

### Change Type

- [x] `major` — Breaking change

### Test Plan

1. Set styles when shapes are not selected.
2. Set styles when shapes are selected.
3. Set styles when shapes are selected and the selected tool is not
select.

- [x] Unit Tests
This commit is contained in:
Steve Ruiz 2023-08-23 12:14:49 +02:00 committed by GitHub
parent 55bd2b2485
commit 2c7c97af9c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 190 additions and 122 deletions

View file

@ -90,7 +90,8 @@ const InsideOfEditorContext = () => {
const interval = setInterval(() => { const interval = setInterval(() => {
const selection = [...editor.selectedShapeIds] const selection = [...editor.selectedShapeIds]
editor.selectAll() editor.selectAll()
editor.setStyle(DefaultColorStyle, i % 2 ? 'blue' : 'light-blue') editor.setStyleForSelectedShapes(DefaultColorStyle, i % 2 ? 'blue' : 'light-blue')
editor.setStyleForNextShapes(DefaultColorStyle, i % 2 ? 'blue' : 'light-blue')
editor.setSelectedShapes(selection) editor.setSelectedShapes(selection)
i++ i++
}, 1000) }, 1000)

View file

@ -13,7 +13,14 @@ export const FilterStyleUi = track(function FilterStyleUi() {
filter:{' '} filter:{' '}
<select <select
value={filterStyle.type === 'mixed' ? 'mixed' : filterStyle.value} value={filterStyle.type === 'mixed' ? 'mixed' : filterStyle.value}
onChange={(e) => editor.setStyle(MyFilterStyle, e.target.value)} onChange={(e) => {
editor.batch(() => {
if (editor.isIn('select')) {
editor.setStyleForSelectedShapes(MyFilterStyle, e.target.value)
}
editor.setStyleForNextShapes(MyFilterStyle, e.target.value)
})
}}
> >
<option value="mixed" disabled> <option value="mixed" disabled>
Mixed Mixed

View file

@ -815,9 +815,11 @@ export class Editor extends EventEmitter<TLEventMap> {
setFocusedGroup(shape: null | TLGroupShape | TLShapeId): this; setFocusedGroup(shape: null | TLGroupShape | TLShapeId): this;
setHintingShapes(shapes: TLShape[] | TLShapeId[]): this; setHintingShapes(shapes: TLShape[] | TLShapeId[]): this;
setHoveredShape(shape: null | TLShape | TLShapeId): this; setHoveredShape(shape: null | TLShape | TLShapeId): this;
setOpacity(opacity: number, historyOptions?: TLCommandHistoryOptions): this; setOpacityForNextShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this;
setOpacityForSelectedShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this;
setSelectedShapes(shapes: TLShape[] | TLShapeId[], historyOptions?: TLCommandHistoryOptions): this; setSelectedShapes(shapes: TLShape[] | TLShapeId[], historyOptions?: TLCommandHistoryOptions): this;
setStyle<T>(style: StyleProp<T>, value: T, historyOptions?: TLCommandHistoryOptions): this; setStyleForNextShapes<T>(style: StyleProp<T>, value: T, historyOptions?: TLCommandHistoryOptions): this;
setStyleForSelectedShapes<T>(style: StyleProp<T>, value: T, historyOptions?: TLCommandHistoryOptions): this;
shapeUtils: { shapeUtils: {
readonly [K in string]?: ShapeUtil<TLUnknownShape>; readonly [K in string]?: ShapeUtil<TLUnknownShape>;
}; };

View file

@ -1210,7 +1210,7 @@ export class Editor extends EventEmitter<TLEventMap> {
partial: Partial<Omit<TLInstance, 'currentPageId'>>, partial: Partial<Omit<TLInstance, 'currentPageId'>>,
historyOptions?: TLCommandHistoryOptions historyOptions?: TLCommandHistoryOptions
) => { ) => {
const prev = this.instanceState const prev = this.store.get(this.instanceState.id)!
const next = { ...prev, ...partial } const next = { ...prev, ...partial }
return { return {
@ -7246,74 +7246,79 @@ export class Editor extends EventEmitter<TLEventMap> {
} }
/** /**
* Set the current opacity. This will effect any selected shapes, or the * Set the opacity for the next shapes. This will effect subsequently created shapes.
* next-created shape.
* *
* @example * @example
* ```ts * ```ts
* editor.setOpacity(0.5) * editor.setOpacityForNextShapes(0.5)
* editor.setOpacity(0.5, { squashing: true }) * editor.setOpacityForNextShapes(0.5, { squashing: true })
* ``` * ```
* *
* @param opacity - The opacity to set. Must be a number between 0 and 1 inclusive. * @param opacity - The opacity to set. Must be a number between 0 and 1 inclusive.
* @param historyOptions - The history options for the change. * @param historyOptions - The history options for the change.
*/ */
setOpacity(opacity: number, historyOptions?: TLCommandHistoryOptions): this { setOpacityForNextShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this {
this.history.batch(() => { this.updateInstanceState({ opacityForNextShape: opacity }, historyOptions)
if (this.isIn('select')) { return this
const { }
currentPageState: { selectedShapeIds },
} = this
const shapesToUpdate: TLShape[] = [] /**
* Set the current opacity. This will effect any selected shapes.
*
* @example
* ```ts
* editor.setOpacityForSelectedShapes(0.5)
* editor.setOpacityForSelectedShapes(0.5, { squashing: true })
* ```
*
* @param opacity - The opacity to set. Must be a number between 0 and 1 inclusive.
* @param historyOptions - The history options for the change.
*/
setOpacityForSelectedShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this {
const { selectedShapes } = this
// We can have many deep levels of grouped shape if (selectedShapes.length > 0) {
// Making a recursive function to look through all the levels const shapesToUpdate: TLShape[] = []
const addShapeById = (id: TLShape['id']) => {
const shape = this.getShape(id) // We can have many deep levels of grouped shape
if (!shape) return // Making a recursive function to look through all the levels
if (this.isShapeOfType<TLGroupShape>(shape, 'group')) { const addShapeById = (shape: TLShape) => {
const childIds = this.getSortedChildIdsForParent(id) if (this.isShapeOfType<TLGroupShape>(shape, 'group')) {
for (const childId of childIds) { const childIds = this.getSortedChildIdsForParent(shape)
addShapeById(childId) for (const childId of childIds) {
} addShapeById(this.getShape(childId)!)
} else {
shapesToUpdate.push(shape)
} }
} } else {
shapesToUpdate.push(shape)
if (selectedShapeIds.length > 0) {
for (const id of selectedShapeIds) {
addShapeById(id)
}
this.updateShapes(
shapesToUpdate.map((shape) => {
return {
id: shape.id,
type: shape.type,
opacity,
}
}),
historyOptions
)
} }
} }
this.updateInstanceState({ opacityForNextShape: opacity }, historyOptions) for (const id of selectedShapes) {
}) addShapeById(id)
}
this.updateShapes(
shapesToUpdate.map((shape) => {
return {
id: shape.id,
type: shape.type,
opacity,
}
}),
historyOptions
)
}
return this return this
} }
/** /**
* Set the value of a {@link @tldraw/tlschema#StyleProp}. This change will be applied to any * Set the value of a {@link @tldraw/tlschema#StyleProp} for the selected shapes.
* selected shapes, and any subsequently created shapes.
* *
* @example * @example
* ```ts * ```ts
* editor.setStyle(DefaultColorStyle, 'red') * editor.setStyleForSelectedShapes(DefaultColorStyle, 'red')
* editor.setStyle(DefaultColorStyle, 'red', { ephemeral: true }) * editor.setStyleForSelectedShapes(DefaultColorStyle, 'red', { ephemeral: true })
* ``` * ```
* *
* @param style - The style to set. * @param style - The style to set.
@ -7322,61 +7327,87 @@ export class Editor extends EventEmitter<TLEventMap> {
* *
* @public * @public
*/ */
setStyle<T>(style: StyleProp<T>, value: T, historyOptions?: TLCommandHistoryOptions): this { setStyleForNextShapes<T>(
this.history.batch(() => { style: StyleProp<T>,
if (this.isIn('select')) { value: T,
const { historyOptions?: TLCommandHistoryOptions
currentPageState: { selectedShapeIds }, ): this {
} = this const {
instanceState: { stylesForNextShape },
} = this
if (selectedShapeIds.length > 0) { this.updateInstanceState(
const updates: { { stylesForNextShape: { ...stylesForNextShape, [style.id]: value } },
util: ShapeUtil historyOptions
originalShape: TLShape )
updatePartial: TLShapePartial
}[] = []
// We can have many deep levels of grouped shape return this
// Making a recursive function to look through all the levels }
const addShapeById = (id: TLShape['id']) => {
const shape = this.getShape(id) /**
if (!shape) return * Set the value of a {@link @tldraw/tlschema#StyleProp}. This change will be applied to the currently selected shapes.
if (this.isShapeOfType<TLGroupShape>(shape, 'group')) { *
const childIds = this.getSortedChildIdsForParent(id) * @example
for (const childId of childIds) { * ```ts
addShapeById(childId) * editor.setStyleForSelectedShapes(DefaultColorStyle, 'red')
} * editor.setStyleForSelectedShapes(DefaultColorStyle, 'red', { ephemeral: true })
} else { * ```
const util = this.getShapeUtil(shape) *
const stylePropKey = this.styleProps[shape.type].get(style) * @param style - The style to set.
if (stylePropKey) { * @param value - The value to set.
const shapePartial: TLShapePartial = { * @param historyOptions - (optional) The history options for the change.
id: shape.id, *
type: shape.type, * @public
props: { [stylePropKey]: value }, */
} setStyleForSelectedShapes<T>(
updates.push({ style: StyleProp<T>,
util, value: T,
originalShape: shape, historyOptions?: TLCommandHistoryOptions
updatePartial: shapePartial, ): this {
}) const { selectedShapes } = this
}
if (selectedShapes.length > 0) {
const updates: {
util: ShapeUtil
originalShape: TLShape
updatePartial: TLShapePartial
}[] = []
// We can have many deep levels of grouped shape
// Making a recursive function to look through all the levels
const addShapeById = (shape: TLShape) => {
if (this.isShapeOfType<TLGroupShape>(shape, 'group')) {
const childIds = this.getSortedChildIdsForParent(shape.id)
for (const childId of childIds) {
addShapeById(this.getShape(childId)!)
}
} else {
const util = this.getShapeUtil(shape)
const stylePropKey = this.styleProps[shape.type].get(style)
if (stylePropKey) {
const shapePartial: TLShapePartial = {
id: shape.id,
type: shape.type,
props: { [stylePropKey]: value },
} }
updates.push({
util,
originalShape: shape,
updatePartial: shapePartial,
})
} }
for (const id of selectedShapeIds) {
addShapeById(id)
}
this.updateShapes(
updates.map(({ updatePartial }) => updatePartial),
historyOptions
)
} }
} }
this.updateInstanceState({ stylesForNextShape: { [style.id]: value } }, historyOptions) for (const shape of selectedShapes) {
}) addShapeById(shape)
}
this.updateShapes(
updates.map(({ updatePartial }) => updatePartial),
historyOptions
)
}
return this return this
} }

View file

@ -94,9 +94,14 @@ function useStyleChangeCallback() {
const editor = useEditor() const editor = useEditor()
return React.useMemo(() => { return React.useMemo(() => {
return function <T>(style: StyleProp<T>, value: T, squashing: boolean) { return function handleStyleChange<T>(style: StyleProp<T>, value: T, squashing: boolean) {
editor.setStyle(style, value, { squashing }) editor.batch(() => {
editor.updateInstanceState({ isChangingStyle: true }) if (editor.isIn('select')) {
editor.setStyleForSelectedShapes(style, value, { squashing })
}
editor.setStyleForNextShapes(style, value, { squashing })
editor.updateInstanceState({ isChangingStyle: true })
})
} }
}, [editor]) }, [editor])
} }
@ -118,8 +123,13 @@ function CommonStylePickerSet({
const handleOpacityValueChange = React.useCallback( const handleOpacityValueChange = React.useCallback(
(value: number, ephemeral: boolean) => { (value: number, ephemeral: boolean) => {
const item = tldrawSupportedOpacities[value] const item = tldrawSupportedOpacities[value]
editor.setOpacity(item, { ephemeral }) editor.batch(() => {
editor.updateInstanceState({ isChangingStyle: true }) if (editor.isIn('select')) {
editor.setOpacityForSelectedShapes(item, { ephemeral })
}
editor.setOpacityForNextShapes(item, { ephemeral })
editor.updateInstanceState({ isChangingStyle: true })
})
}, },
[editor] [editor]
) )

View file

@ -150,7 +150,8 @@ it('Does not create an undo stack item when first clicking on an empty canvas',
describe('Editor.sharedOpacity', () => { describe('Editor.sharedOpacity', () => {
it('should return the current opacity', () => { it('should return the current opacity', () => {
expect(editor.sharedOpacity).toStrictEqual({ type: 'shared', value: 1 }) expect(editor.sharedOpacity).toStrictEqual({ type: 'shared', value: 1 })
editor.setOpacity(0.5) editor.setOpacityForSelectedShapes(0.5)
editor.setOpacityForNextShapes(0.5)
expect(editor.sharedOpacity).toStrictEqual({ type: 'shared', value: 0.5 }) expect(editor.sharedOpacity).toStrictEqual({ type: 'shared', value: 0.5 })
}) })
@ -197,7 +198,8 @@ describe('Editor.setOpacity', () => {
]) ])
editor.setSelectedShapes([ids.A, ids.B]) editor.setSelectedShapes([ids.A, ids.B])
editor.setOpacity(0.5) editor.setOpacityForSelectedShapes(0.5)
editor.setOpacityForNextShapes(0.5)
expect(editor.getShape(ids.A)!.opacity).toBe(0.5) expect(editor.getShape(ids.A)!.opacity).toBe(0.5)
expect(editor.getShape(ids.B)!.opacity).toBe(0.5) expect(editor.getShape(ids.B)!.opacity).toBe(0.5)
@ -216,7 +218,8 @@ describe('Editor.setOpacity', () => {
]) ])
editor.setSelectedShapes([ids.groupA]) editor.setSelectedShapes([ids.groupA])
editor.setOpacity(0.5) editor.setOpacityForSelectedShapes(0.5)
editor.setOpacityForNextShapes(0.5)
// a wasn't selected... // a wasn't selected...
expect(editor.getShape(ids.boxA)!.opacity).toBe(1) expect(editor.getShape(ids.boxA)!.opacity).toBe(1)
@ -232,9 +235,11 @@ describe('Editor.setOpacity', () => {
}) })
it('stores opacity on opacityForNextShape', () => { it('stores opacity on opacityForNextShape', () => {
editor.setOpacity(0.5) editor.setOpacityForSelectedShapes(0.5)
editor.setOpacityForNextShapes(0.5)
expect(editor.instanceState.opacityForNextShape).toBe(0.5) expect(editor.instanceState.opacityForNextShape).toBe(0.5)
editor.setOpacity(0.6) editor.setOpacityForSelectedShapes(0.6)
editor.setOpacityForNextShapes(0.6)
expect(editor.instanceState.opacityForNextShape).toBe(0.6) expect(editor.instanceState.opacityForNextShape).toBe(0.6)
}) })
}) })

View file

@ -92,14 +92,15 @@ it('Creates shapes with the current style', () => {
editor.createShapes([{ id: ids.box1, type: 'geo' }]) editor.createShapes([{ id: ids.box1, type: 'geo' }])
expect(editor.getShape<TLGeoShape>(ids.box1)!.props.color).toEqual('black') expect(editor.getShape<TLGeoShape>(ids.box1)!.props.color).toEqual('black')
editor.setStyle(DefaultColorStyle, 'red') editor.setStyleForSelectedShapes(DefaultColorStyle, 'red')
editor.setStyleForNextShapes(DefaultColorStyle, 'red')
expect(editor.instanceState.stylesForNextShape[DefaultColorStyle.id]).toBe('red') expect(editor.instanceState.stylesForNextShape[DefaultColorStyle.id]).toBe('red')
editor.createShapes([{ id: ids.box2, type: 'geo' }]) editor.createShapes([{ id: ids.box2, type: 'geo' }])
expect(editor.getShape<TLGeoShape>(ids.box2)!.props.color).toEqual('red') expect(editor.getShape<TLGeoShape>(ids.box2)!.props.color).toEqual('red')
}) })
it('Creates shapes with the current opacity', () => { it('Creates shapes with the current opacity', () => {
editor.setOpacity(0.5) editor.setOpacityForNextShapes(0.5)
editor.createShapes([{ id: ids.box3, type: 'geo' }]) editor.createShapes([{ id: ids.box3, type: 'geo' }])
expect(editor.getShape<TLGeoShape>(ids.box3)!.opacity).toEqual(0.5) expect(editor.getShape<TLGeoShape>(ids.box3)!.opacity).toEqual(0.5)
}) })

View file

@ -11,7 +11,8 @@ const ids = {
beforeEach(() => { beforeEach(() => {
editor = new TestEditor() editor = new TestEditor()
editor.setStyle(DefaultDashStyle, 'solid') editor.setStyleForNextShapes(DefaultDashStyle, 'solid')
editor.setStyleForSelectedShapes(DefaultDashStyle, 'solid')
editor.createShapes([ editor.createShapes([
{ {
id: ids.boxA, id: ids.boxA,

View file

@ -494,7 +494,8 @@ describe('frame shapes', () => {
.pointerDown(125, 125) .pointerDown(125, 125)
.pointerMove(175, 175) .pointerMove(175, 175)
.pointerUp(175, 175) .pointerUp(175, 175)
.setStyle(DefaultFillStyle, 'solid') .setStyleForSelectedShapes(DefaultFillStyle, 'solid')
.setStyleForNextShapes(DefaultFillStyle, 'solid')
const boxId = editor.onlySelectedShape!.id const boxId = editor.onlySelectedShape!.id
editor.setCurrentTool('arrow') editor.setCurrentTool('arrow')
@ -607,7 +608,8 @@ describe('frame shapes', () => {
.pointerDown(150, 150) .pointerDown(150, 150)
.pointerMove(400, 400) .pointerMove(400, 400)
.pointerUp(400, 400) .pointerUp(400, 400)
.setStyle(DefaultFillStyle, 'solid') .setStyleForSelectedShapes(DefaultFillStyle, 'solid')
.setStyleForNextShapes(DefaultFillStyle, 'solid')
const innerBoxId = editor.onlySelectedShape!.id const innerBoxId = editor.onlySelectedShape!.id
// Make an arrow that binds to the inner box's bottom right corner // Make an arrow that binds to the inner box's bottom right corner
@ -660,7 +662,8 @@ test('arrows bound to a shape within a group within a frame are reparented if th
.pointerDown(110, 110) .pointerDown(110, 110)
.pointerMove(120, 120) .pointerMove(120, 120)
.pointerUp(120, 120) .pointerUp(120, 120)
.setStyle(DefaultFillStyle, 'solid') .setStyleForSelectedShapes(DefaultFillStyle, 'solid')
.setStyleForNextShapes(DefaultFillStyle, 'solid')
const boxAId = editor.onlySelectedShape!.id const boxAId = editor.onlySelectedShape!.id
editor.setCurrentTool('geo') editor.setCurrentTool('geo')
@ -668,7 +671,8 @@ test('arrows bound to a shape within a group within a frame are reparented if th
.pointerDown(180, 110) .pointerDown(180, 110)
.pointerMove(190, 120) .pointerMove(190, 120)
.pointerUp(190, 120) .pointerUp(190, 120)
.setStyle(DefaultFillStyle, 'solid') .setStyleForSelectedShapes(DefaultFillStyle, 'solid')
.setStyleForNextShapes(DefaultFillStyle, 'solid')
const boxBId = editor.onlySelectedShape!.id const boxBId = editor.onlySelectedShape!.id
editor.setCurrentTool('geo') editor.setCurrentTool('geo')
@ -676,7 +680,8 @@ test('arrows bound to a shape within a group within a frame are reparented if th
.pointerDown(160, 160) .pointerDown(160, 160)
.pointerMove(170, 170) .pointerMove(170, 170)
.pointerUp(170, 170) .pointerUp(170, 170)
.setStyle(DefaultFillStyle, 'solid') .setStyleForSelectedShapes(DefaultFillStyle, 'solid')
.setStyleForNextShapes(DefaultFillStyle, 'solid')
const boxCId = editor.onlySelectedShape!.id const boxCId = editor.onlySelectedShape!.id
editor.setCurrentTool('select') editor.setCurrentTool('select')

View file

@ -1919,7 +1919,8 @@ describe('Group opacity', () => {
it("should set the group's opacity to max even if the selected style panel opacity is lower", () => { it("should set the group's opacity to max even if the selected style panel opacity is lower", () => {
editor.createShapes([box(ids.boxA, 0, 0), box(ids.boxB, 20, 0)]) editor.createShapes([box(ids.boxA, 0, 0), box(ids.boxB, 20, 0)])
editor.select(ids.boxA, ids.boxB) editor.select(ids.boxA, ids.boxB)
editor.setOpacity(0.5) editor.setOpacityForSelectedShapes(0.5)
editor.setOpacityForNextShapes(0.5)
editor.groupShapes(editor.selectedShapeIds) editor.groupShapes(editor.selectedShapeIds)
const group = editor.getShape(onlySelectedId())! const group = editor.getShape(onlySelectedId())!
assert(editor.isShapeOfType<TLGroupShape>(group, 'group')) assert(editor.isShapeOfType<TLGroupShape>(group, 'group'))

View file

@ -152,7 +152,8 @@ describe('Editor.setStyle', () => {
]) ])
editor.setSelectedShapes([ids.A, ids.B]) editor.setSelectedShapes([ids.A, ids.B])
editor.setStyle(DefaultColorStyle, 'red') editor.setStyleForSelectedShapes(DefaultColorStyle, 'red')
editor.setStyleForNextShapes(DefaultColorStyle, 'red')
expect(editor.getShape<TLGeoShape>(ids.A)!.props.color).toBe('red') expect(editor.getShape<TLGeoShape>(ids.A)!.props.color).toBe('red')
expect(editor.getShape<TLGeoShape>(ids.B)!.props.color).toBe('red') expect(editor.getShape<TLGeoShape>(ids.B)!.props.color).toBe('red')
@ -171,7 +172,8 @@ describe('Editor.setStyle', () => {
]) ])
editor.setSelectedShapes([ids.groupA]) editor.setSelectedShapes([ids.groupA])
editor.setStyle(DefaultColorStyle, 'red') editor.setStyleForSelectedShapes(DefaultColorStyle, 'red')
editor.setStyleForNextShapes(DefaultColorStyle, 'red')
// a wasn't selected... // a wasn't selected...
expect(editor.getShape<TLGeoShape>(ids.boxA)!.props.color).toBe('black') expect(editor.getShape<TLGeoShape>(ids.boxA)!.props.color).toBe('black')
@ -187,9 +189,11 @@ describe('Editor.setStyle', () => {
}) })
it('stores styles on stylesForNextShape', () => { it('stores styles on stylesForNextShape', () => {
editor.setStyle(DefaultColorStyle, 'red') editor.setStyleForSelectedShapes(DefaultColorStyle, 'red')
editor.setStyleForNextShapes(DefaultColorStyle, 'red')
expect(editor.instanceState.stylesForNextShape[DefaultColorStyle.id]).toBe('red') expect(editor.instanceState.stylesForNextShape[DefaultColorStyle.id]).toBe('red')
editor.setStyle(DefaultColorStyle, 'green') editor.setStyleForSelectedShapes(DefaultColorStyle, 'green')
editor.setStyleForNextShapes(DefaultColorStyle, 'green')
expect(editor.instanceState.stylesForNextShape[DefaultColorStyle.id]).toBe('green') expect(editor.instanceState.stylesForNextShape[DefaultColorStyle.id]).toBe('green')
}) })
}) })