Fall back to default block styles when a new block is being inserted#22167
Fall back to default block styles when a new block is being inserted#22167adamziel wants to merge 3 commits into
Conversation
|
Size Change: +1.96 kB (0%) Total Size: 824 kB
ℹ️ View Unchanged
|
|
Can you give a more detailed example of the issue you are trying to solve? It would help to see the list of style variations set and the generated output before and after. |
| * @param {Object} state Data state. | ||
| * @param {string} name Block type name. | ||
| * | ||
| * @return {Array?} Block Styles. |
| * @return {Array?} Block Styles. | ||
| */ | ||
| export function getDefaultBlockStyle( state, name ) { | ||
| return getBlockStyles( state, name ).filter( |
There was a problem hiding this comment.
find from Lodash might be safer because of filter on undefined (@return {Array?} Block Styles.) will throw an error.
Well, optional chaining might work :)
|
I'm a bit unclear why we're adding a white background to the Navigation block. One common user experience with Navs is to add them on top of other backgrounds in a Header. Here's an example: https://cloudup.com/cfkiaDLkbij If we do default a background color, we should allow that background to be removed. This doesn't seem to happen currently.
|
|
@mapk we need a background color otherwise we get the transparency issues in the navigation block. I would expect that the 'clear' would do just that and it highlights the background color here:
That should solve the unexpected situation. |
|
The only transparency issues I've noticed in the Nav block have to do with the subnav items. Are there others? I'm sure there's a way we can include a background color on the subnav items, but not on the main block. I tried clicking "clear" in my video and it didn't do anything. 😄 |
|
@gziolo The culprit is the following line: Without this PR, it only considers gutenberg/packages/block-library/src/navigation/index.js Lines 80 to 83 in 05c3931 Without this PR, a new navigation block would get no variation assigned even though
@karmatosed @mapk good note! I think this is consideration is separate from this PR - IMHO the default style variation should be assigned to newly created blocks by default. Whether or not the default menu variation is For example maybe we should have a third variation? As in transparent background with white sub-menus.
The |
|
Do you mean that the class name is not injected into generated HTML? If yes, it’s a design decision that was made and it’s expected behavior. Only non-default style variations cause new class name to be added. @youknowriad or @mtias know the full story. |
|
I feel dark and white should not affect the background of the main navigation container, only the submenus. |
|
Let's move the background color discussion to a dedicated PR here: #22228
That's what I meant, yes! Hm, but if I switch to a dark variation and then switch back to the light style, the class is going to be added - should it be not added in that case then? |
|
Closing these PR as per last @gziolo comment |
Description
When you call
insertBlocksorreplaceBlocks, they would add default styles to all the blocks being inserted. At the moment, only preferredStyleVariations are considered, and if those are missing then no style variation is set at all. This PR makes sure we fall back to default style variation from the block definition.For example, the navigation block has the following style variations:
gutenberg/packages/block-library/src/navigation/index.js
Lines 80 to 83 in 05c3931
Without this PR, a new navigation block would get no variation assigned even though
lightis supposed to be the default one - so the background ends up being transparent.Before
After
How has this been tested?
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: