fix(pte): update to allow block styles to be hidden #6379
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
No changes to documentation |
|
Component Testing Report Updated May 22, 2024 12:59 PM (UTC)
|
| @@ -108,13 +108,17 @@ function ensureNormalStyle(styles: any) { | |||
| : [BLOCK_STYLES.normal, ...styles] | |||
| } | |||
|
|
|||
| function filterHiddenStyles(styles: any) { | |||
| return styles.filter((style: any) => !style.hidden) | |||
There was a problem hiding this comment.
COULD: this would allow for any and all styles to be removed, is that ok? Or if the requirement is only to allow for hiding normal style at this stage might be able to check for this in the filter also?
There was a problem hiding this comment.
Great question! It seemed odd to only allow hidden in the 'normal' style. Yes, I think potentially someone could remove all styles, but in practice, would they and what would that mean?
jtpetty
requested review from
RitaDias and
binoy14
and removed request for
a team
There was a problem hiding this comment.
This looks like a good approach!
I think that I would like to hear some thoughts from someone on the design side about what they think about the UX of it apart from my thoughts on it :)
We should also add this to documentation so that more people are aware that they can do this 👍
|
@jtpetty Fred brought up something on slack which might be worth having a look at. In instances where you create a new block (with already some text there) then the new text will be on the "normal style" and a new item will be added to the dropdown called "no style". I think this is something we should account for / discuss on what we want to happen then :) You can see the example and thread here |
Description
Fixes EDX-668
By default, we explicitly add in an entry for a Normal block style. This PR adds an option to hide block styles since there is no way to exclude a Normal style
Before:
After:
What to review
I am not sure if this is the best way to solve this issue. This seemed to be the most natural since we were explicitly adding Normal styles before and I assume that was done intentionally. Removing that would be a breaking change. I also didn't want to add another property
noDefaultNormalor something like that. Open to other suggestions.Testing
Manual testing for now, but will add tests once the approach is validated.
Notes for release