Account created on 6 July 2012, about 12 years ago
#

Merge Requests

More

Recent comments

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Merged to 3.0.x. I'll keep this issue open if someone wants to do a MR for 8.x-2.x.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

We're having issues on Gitlab CI, specifically on the phpunit (max PHP version) job:

Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The configuration key "autoconfigure" cannot be used to define a default value in "modules/custom/gutenberg-3456995/gutenberg.services.yml". Allowed keys are "public", "tags", "autowire".

But it succeeds on the other phpunit jobs πŸ€”

πŸ‡΅πŸ‡ΉPortugal marcofernandes

@alexpott That'll work. Thanks.
Note that also adding the tags to the gutenberg.media_library.opener service would also work.

  gutenberg.media_library.opener:
    class: Drupal\gutenberg\MediaLibraryGutenbergOpener
    tags:
      - { name: media_library.opener }

But, I guess, this way, we don't have to worry about other autoconfigurable services that may come up.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Btw @doxigo, you're observing this on the editor page (node editing), right? Because Gutenberg JS files shouldn't be loaded on frontend.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Although there's those webpack comments (only on JS files), we're using a "production" build. It can be compared to the WP Gutenberg plugin: https://wordpress.org/plugins/gutenberg/
The packages are generated by core's Gutenberg build script.
CSS files don't have any comments.

I'm not sure if it's worth the time to change/replace core Gutenberg's build script to remove those webpack comments just to make Lighthouse happy on a backend page. πŸ˜…

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Gutenberg Cloud was discontinued (no longer available on 3+). On 2.x will display empty because the the API is no longer available.
If anyone interested in continuing it, just take the code from 2.x and create a new module.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

@doxigo Yeah, unfortunately d.o is a bit strict with branches naming/versioning. The idea of having 3.x branch was to avoid managing minor versions. I merged 3.x into 3.0.x and have set 3.0.x back to default.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

If anyone could rebase this issue branch with the latest 2.x I would appreciate it.
I would be even more grateful if this went to 3.x also. 😎

πŸ‡΅πŸ‡ΉPortugal marcofernandes

@bradjones1 Yes, I understand that πŸ˜‰ but my comment was meant as a heads-up for people who are still in the 9.x and comes across this issue. The solution was just remove that rejected hunk.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Hi, I'm trying to apply a patch based on MR183 to 9.5.x but it's failing at TestRunnerKernel.php

➜  drupal git:(9.5.x) βœ— patch -p1 < ./patches/183.patch
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/modules/system/tests/modules/router_test_directory/router_test.module'
patching file 'core/modules/system/tests/modules/router_test_directory/router_test.routing.yml'
patching file 'core/modules/system/tests/modules/router_test_directory/src/RouterTestEarlyExceptionSubscriber.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/RouterTestServiceProvider.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/TestControllers.php'
patching file 'core/tests/Drupal/FunctionalTests/Routing/RouteCachingQueryAlteredTest.php'
patching file 'core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php'
patching file 'core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/RouterTestEarlyExceptionSubscriber.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/RouterTestEarlyExceptionSubscriber.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/TestControllers.php'
patching file 'core/tests/Drupal/FunctionalTests/Routing/RouteCachingQueryAlteredTest.php'
patching file 'core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/RouterTestEarlyExceptionSubscriber.php'
patching file 'core/tests/Drupal/FunctionalTests/Routing/RouteCachingQueryAlteredTest.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/lib/Drupal/Core/Test/TestRunnerKernel.php'
1 out of 1 hunks failed--saving rejects to 'core/lib/Drupal/Core/Test/TestRunnerKernel.php.rej'
patching file 'core/modules/system/tests/modules/router_test_directory/src/RouterTestEarlyExceptionSubscriber.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/modules/system/tests/modules/router_test_directory/src/TestControllers.php'
patching file 'core/lib/Drupal/Core/Routing/RouteProvider.php'
patching file 'core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php'
➜  drupal git:(9.5.x) βœ— 

Maybe related to the failed test on this commit?: https://git.drupalcode.org/project/drupal/-/merge_requests/183/diffs?com...

πŸ‡΅πŸ‡ΉPortugal marcofernandes

I read all you monologue here, @loze πŸ˜… It was great to follow your thinking.
The table for block usage seems the best approach.
IIRC, Layout Builder when saving the layout of a node with revisions will create new content blocks even if the content block type has revisions disabled. Maybe we should follow the same approach? Because IMO since the content blocks are tied to nodes, there's no need to handle block revisions. But we could check how LB handles the insert/update/delete operations regarding content blocks.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Reusable blocks / Patterns issue should be solved by this πŸ“Œ Patterns and synced patterns (reusable blocks) Needs work

I'll take a look at the typography issues but note that font sizes definition location has changed. For example:

theme-support:
  __experimentalFeatures:
    typography:
      fontSizes:
        theme:
          - name: Small
            size: "0.8125em"
            slug: small
          - name: Medium
            size: "1em"
            slug: medium
          - name: Large
            size: "1.50em"
            slug: large
          - name: Extra Large
            size: "2em"
            slug: extra-large
          - name: Extra Extra Large
            size: "4em"
            slug: extra-extra-large

To enable dimensions you need to set the following configuration to theme's configuration file:

theme-support:
  __experimentalFeatures:
    spacing:
      margin: true
      padding: true
      units:
        - px
        - em
        - rem

I couldn't replicate the custom font size, at least with Olivero but make sure the option is enabled at:

theme-support:
  __experimentalFeatures:
    typography:
      customFontSize: true

I suggest to take a look at the configuration from the Starter theme: https://git.drupalcode.org/project/gutenberg_starter/-/blob/1.0.x/gutenb...
Still WIP but a good reference.

Btw, some configuration definitions have changed without a deprecation step because of the "long" jump we made regarding the core library. In future releases we will do better πŸ˜‰

πŸ‡΅πŸ‡ΉPortugal marcofernandes

I tested updating the latest version of core lib and the issue persists. Needs more debugging.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Nicely done! Now implemented in the proper way πŸ˜‰

πŸ‡΅πŸ‡ΉPortugal marcofernandes

IIRC the current example on the gutenberg.api file was made for Group 2.x. I guess it makes sense to update it to 3.x.
Could you provide a MR for it? It would be easier to review πŸ˜‰

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Hmm... I don't think it's enough to just clear the contentBlockId attribute.

Here's my thoughts on how it should work:
When duplicating a content block or copy/paste it, at Drupal side, a new content block must be created (cloned from the source) an pass the contentBlockIdto the gutenberg block.

Unfortunately there isn't a filter/hook to handle the duplicate action nor copy/paste https://stackoverflow.com/questions/67667923/how-to-detect-when-a-block-....
A possible implementation would be creating a filter for editor.BlockEdit (example at /filters/mapping-fields.jsx) to somehow handle the content block duplication.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Not easy to fit a visual indicator for the required mapping fields. And I mean that in a styling perspective. I tried the usual (*), a red outline border and even a different background color for the block but I didn't like the results.

I made some changes so the mapping fields filter is ready to handle the required fields but, for now, I just added at the sidebar a text telling that the field is required πŸ€·β€β™‚οΈ

We'll revisit this with some designers/UX experts opinions πŸ˜‰

For now, I'm happy with the MR. Good work and thank you @loze.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

marcofernandes β†’ made their first commit to this issue’s fork.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

I avoid adding exceptions for other non-core modules but in this case we don't have much choice.
And this just reminded me that we need to simplify a lot our integration with Media module. Maybe follow CKEditor approach if possible.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Took the opportunity to make some changes regarding some Dependency Injections missing.
Good work πŸ˜‰

πŸ‡΅πŸ‡ΉPortugal marcofernandes

marcofernandes β†’ made their first commit to this issue’s fork.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

No, you're not going it wrong and it's possible to do it the right way πŸ˜‰

Take a look at the shinny new example blocks sub-module at 3.x branch:
https://git.drupalcode.org/project/gutenberg/-/tree/3.x/modules/example_...

It's using wp-scripts package and it deals with wp packages imports, it won't add them to the bundle and it'll change those imports to global objects.

Check it out and let me know if you need more help.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

I'm glad the starter theme is helping, that's what it was made for πŸ˜…

πŸ‡΅πŸ‡ΉPortugal marcofernandes

I couldn't replicate but you're right about the 'view_mode' index usage at block_content_theme_suggestions_block_alter().
I don't think there's a title property for content blocks but a description (the reference is info). But I think it's irrelevant - it will be always the same in case of block instances created by Gutenberg.
Let's just add your patch.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

I'm not sure how we could handle this in a more "robust" way but, yes, it seems simpler to just check for the url.
Good find and thank you for the MR πŸ‘πŸΌ

πŸ‡΅πŸ‡ΉPortugal marcofernandes

@loze I couldn't replicate but I did find some test code that might have caused that error.
Could you provide more details about your site install? Drupal core version, installed modules/themes?

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Oh.. I put the hyphens for better readability but now I see it just added confusion. Sorry about that. Anyway, I'm glad it work.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Btw, I'm changing this issue classification because it's not a bug nor critical. It is, after all, a user behaviour. It's bad UX, for sure.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Sorry 😬, but I think we shouldn't go for that approach. Gutenberg hook filters should definitely be used here.

And I'm not really convinced that we should adopted this behaviour for 3.x since now the editor needs to select the reusable block (synced pattern) in order to edit it. And when selecting a synced pattern, its color is different (magenta). So, I believe, they will be aware they are editing the pattern. πŸ€”

Here's how it works in 3.x:
https://www.drupal.org/files/issues/2024-05-20/Screen%20Recording%202024... β†’

But for 2.x probably makes sense since we don't have almost any visual indication.

πŸ’¬ | Gutenberg | Iframe.ly
πŸ‡΅πŸ‡ΉPortugal marcofernandes

Moving this to normal since we have a alternative by using Media module.
Ideally I would like to avoid a dependency to Media module and try to solve this either with a library or a free service. Feedback is welcome πŸ˜‰
Btw, Youtube embeds uses Youtube's own service and this is defined at the Oembed settings:
/admin/config/content/formats/manage/gutenberg
Also valid for Vimeo and Twitter/X.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Again, due to Meta policies, these embed blocks will be removed. For now the workaround will be using the generic embed block. Closing.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Work has been done that might mitigate/fix this issue like validating if reusable block content are empty. Closing.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Version 2.x has a better support for Drupal blocks and this should work without issues. Closing.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

To make sure your scripts runs after Gutenberg is initialized, add a dependency to gutenberg/edit-node to your library.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Closing this one since no more info added. Anyway, 1.x is not supported.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Since there's no more feedback I'm assuming the cause of this issue was the AdBlock. Closing.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

marcofernandes β†’ made their first commit to this issue’s fork.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

Closing. Feel free to open if you have more info.

πŸ‡΅πŸ‡ΉPortugal marcofernandes

You should try a later 2.x version to see if it still occurs. Closing for now.

Production build 0.69.0 2024