Style presets #1277
Style presets #1277
Conversation
|
Size Change: +1.03 kB (0%) Total Size: 842 kB
ℹ️ View Unchanged
|
Codecov Report
@@ Coverage Diff @@
## master #1277 +/- ##
=========================================
Coverage 56.91% 56.91%
Complexity 218 218
=========================================
Files 514 514
Lines 8505 8505
=========================================
Hits 4841 4841
Misses 3664 3664
|
swissspidy
added
the
Type: Enhancement
| const isStylePreset = preset.backgroundTextMode !== undefined; | ||
| updateElementsById({ | ||
| elementIds: selectedElementIds, | ||
| properties: isStylePreset ? { ...preset } : { color: preset }, |
There was a problem hiding this comment.
/cc @barklund
So, this will break completely with new text styling. Perhaps we should just proceed here and fix once that lands? I don't think it'd be a major update at that point. Instead of updating a color, we would be updating content field.
There was a problem hiding this comment.
Note that the text presets currently already are using color for text which Morten will have to re-work in the new text editing PR anyway. Probably the change coming from this PR won't be that much different to rework compared to what already exists for Color Presets currently, meaning there shouldn't be a huge difference in terms of which lands first from Morten's PR's perspective.
An alternative would be leaving the color out of the Text editing PR for now and handling it together with presets separately to reduce the size of the text editing PR. There might be some UX questions to resolve anyway, e.g.
If the user selects part of the text and clicks on a color preset, should the color be applied to the selection only (and the rest of the attributes to the full element)? If there are multiple colors in an element and the user clicks "Add preset", should a preset be added for each color? Or should a preset be added only when color is applied to the full Text?
That's probably a better discussion under #1323, however, just making a suggestion that perhaps color might be something to handle separately together with Style Presets in a separate PR. Thoughts?
cc @barklund
There was a problem hiding this comment.
IMHO, at this point it will be easier to pursue separately and fix upstream, since both are rather new features.
assets/src/edit-story/components/panels/stylePreset/presetGroup.js
Outdated
Show resolved
Hide resolved
assets/src/edit-story/components/panels/stylePreset/presetGroup.js
Outdated
Show resolved
Hide resolved
assets/src/edit-story/components/panels/stylePreset/test/presetGroup.js
Outdated
Show resolved
Hide resolved
| const isStylePreset = preset.backgroundTextMode !== undefined; | ||
| updateElementsById({ | ||
| elementIds: selectedElementIds, | ||
| properties: isStylePreset ? { ...preset } : { color: preset }, |
Fixes #279
Todo:
textStylessince styles for shapes will be tracked separately.