Montpellier, France
Account created on 16 September 2010, almost 15 years ago
#

Merge Requests

More

Recent comments

🇫🇷France duaelfr Montpellier, France

@julien tekrane Would you provide me your patterns in an attached zip file so I can try to reproduce locally?

🇫🇷France duaelfr Montpellier, France

@julien tekrane: just to be sure I understand, are we talking about settings as in "ui_patterns_settings" (ie. defined in the pattern) or settings as in field_group settings (ie. group label, group shown when empty, etc.)?

I both case, I'm not sure we want that inheritance to be the default behavior, so *if* it was to be implemented, it should be opt-in.

@pdureau What do you think about this? I believe this kind of inheritance never existed and still don't exist in 2.x, is it?

🇫🇷France duaelfr Montpellier, France

duaelfr made their first commit to this issue’s fork.

🇫🇷France duaelfr Montpellier, France

@julien tekrane:
Hi!
Thank you for bringing this back. I recognize a big part of your code as my own code from an old PR on github.
I have been using this for a very long time but it seem that it is not needed anymore to get nested groups working.
I removed this patch on all my projects without any difference. I guess that's because some "recent" changes in Field Group.

I just pushed a tiny change in our field group formatter to ensure that field_group_remove_empty_display_groups() has what it needs to handle our groups. That should fix the last use cases with nested groups.

Do you still have issues? Can you provide steps to reproduce on a clean install?

🇫🇷France duaelfr Montpellier, France

@julien tekrane: I don't get your need here. Your code overwrites the child field group settings by the parent ones. I'm quite sure this is not a behavior we want by default. Could you elaborate a bit on what you are trying to achieve here? Maybe give a specific example.

🇫🇷France duaelfr Montpellier, France

Since your first patch, 🐛 TypeError: Argument 1 passed to Drupal\ui_patterns_views\Plugin\views\row\Pattern::isFieldVisible() Fixed has been committed to prevent TypeError in case of a non-existent field.
I am not able to reproduce your warning but that's true that !isset can throw it. So, I decided to convert this condition to use empty instead.

@jatingupta40 #8: I don't think this patch (or your MR) should be introduced in the module because

  • !empty($field) is already managed by the condition line 44
  • !empty($field_output) is already managed within the isFieldVisible() method which will also preserve the choice about empty values in the configuration

I'll mark this issue as Fixed. Feel free to reopen if needed.

🇫🇷France duaelfr Montpellier, France

@jrockowitz Thank you for your answer.

My current issue is that I am trying to map one of my existing content types, which uses Paragraphs for its main content, to a NewsArticle schema.org type, which expects an articleBody attribute of type Text. When I'm choosing my entity reference revision field, no Text content is rendered in the attribute. If my paragraphs are mapped to a schema.org type, these are rendered as an array in the attribute, what's not the expected type.

Am I completely wrong trying to map this paragraphs based content to this attribute?

🇫🇷France duaelfr Montpellier, France

phpstan should be fixed now but I'm not sure it's the best way...
May I have an ultimate review on this?

🇫🇷France duaelfr Montpellier, France

duaelfr made their first commit to this issue’s fork.

🇫🇷France duaelfr Montpellier, France

@pdureau I rebased the MR on the latest commit.

🇫🇷France duaelfr Montpellier, France

Do you think we could offset elements a bit to the right to create some kind of "nested" effect?
It might help understand where a context starts.


(This obviously need a bit more love but I'm not a designer)

From an accessibility point of view, this "hierarchical" structure would need a more appropriate markup. We might want to use fieldsets and legends to do that (and that would help with the point above).

🇫🇷France duaelfr Montpellier, France

I like goz's second idea and I think this could also work when nothing has been configured yet:

Props

  • URL: Data from field (field_link) Change source for URL
  • Color: -None- Change source for Color

We would have to define a way to display a "summary" of the existing settings but this would be way more effective for performances because we would only need to load forms on demand. The downside is still usability because it's one more click in an UI that already needs a lot.

🇫🇷France duaelfr Montpellier, France

I am very surprised because I never faced this issue.
I just tried to reproduce this following the steps to reproduce in the Issue Summary but everything is working fine.

  1. I installed Drupal 10.3.14 on simplytest.me
  2. I configured the "Basic HTML" text filter to use this module's plugin (but not the "Full HTML" filter)
  3. I copied a bunch of content from https://en.wikipedia.org/wiki/Drupal
  4. I pasted it into my editor in "Full HTML" format, markup was preserved
  5. I pasted the content into my editor in "Basic HTML" format, markup was removed

I tested both Firefox and Chrome in case it was browser dependant but I had the same results.
(See attached screencast)

🇫🇷France duaelfr Montpellier, France

Existing tests are stable. Let's merge this before moving on to the next major.

🇫🇷France duaelfr Montpellier, France

Existing tests are stable. Let's merge this before moving on to the next major.

🇫🇷France duaelfr Montpellier, France

Converted the patch in a MR to let the CI do its magic

🇫🇷France duaelfr Montpellier, France

duaelfr made their first commit to this issue’s fork.

🇫🇷France duaelfr Montpellier, France

Existing tests are stable. Let's merge this before moving on to the next major.

🇫🇷France duaelfr Montpellier, France

Existing tests are stable. Let's merge this before moving on to the next major.

🇫🇷France duaelfr Montpellier, France

This has been fixed in 📌 [1.11.0] Fix CI failures Active to get tests green.
Thanks!

🇫🇷France duaelfr Montpellier, France

This one made my cry blood. I hope next ones will be easier ;)

🇫🇷France duaelfr Montpellier, France

duaelfr created an issue.

🇫🇷France duaelfr Montpellier, France

(Sorry for the above commits. It was a rebase mistake.)

🇫🇷France duaelfr Montpellier, France

Done (no need for a new patch as it's only a typo in a comment).

🇫🇷France duaelfr Montpellier, France

TBH, I don't remember why I needed this such a long time ago.
I just checked 11.x code and this has not been implemented yet.
Creating this new method wouldn't impact existing code and wouldn't need any new dependency. It'd be a pure helper for DX.

Given the lack of comments here. I guess this is not really needed by anyone ;)
Let's see.

🇫🇷France duaelfr Montpellier, France

There is a proposal.
Patch attached for composer.

🇫🇷France duaelfr Montpellier, France

New attempt to fix the issue.
Patch for composer attached.

🇫🇷France duaelfr Montpellier, France

@pdureau Thanks for the review.
Do you have some examples of other locations where patterns can be found? How can this be dynamically determined?

🇫🇷France duaelfr Montpellier, France

There you go.
Patch attached for composer users.

🇫🇷France duaelfr Montpellier, France

FYI, the Audit Files module provides a report that tries to match existing duplicated files and allow to merge them.

🇫🇷France duaelfr Montpellier, France

I needed that on D10 to delay an upgrade so I rerolled and split the patch.

If you need that feature you have to apply:

  • 2741945-103-ckeditor.patch on the ckeditor module
  • 2741945-103-core-10.4.patch on Drupal Core (tested on 10.4 but might also work on previous 10.x versions)
🇫🇷France duaelfr Montpellier, France

This wasn't working on Drupal 10.3+ because of the introduction of the \Drupal\Core\File\FileExists enum (See the related change record ).
Patch attached for composer friends.

🇫🇷France duaelfr Montpellier, France

First shot has been pushed to the dev branch and release in alpha.
It still needs to support nested values, so I'm keeping this issue open.

🇫🇷France duaelfr Montpellier, France

duaelfr created an issue.

🇫🇷France duaelfr Montpellier, France

@lendude @smustgrave I'm out of ideas to fix the test. I cannot reproduce this failure locally. Do you have some clues for me?

$ phpunit -c phpunit.xml core/modules/views/tests/src/Functional/BulkFormTest.php
PHPUnit 10.5.45 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.15
Configuration: /app/phpunit.xml

..                                                                  2 / 2 (100%)

HTML output was generated.
[...]

Time: 00:18.124, Memory: 10.00 MB

OK (2 tests, 79 assertions)
$ phpunit -c phpunit.xml core/modules/workspaces/tests/src/Functional/WorkspaceViewsBulkFormTest.php
PHPUnit 10.5.45 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.15
Configuration: /app/phpunit.xml

..                                                                  2 / 2 (100%)

HTML output was generated.
[...]


Time: 00:56.580, Memory: 10.00 MB

OK (2 tests, 97 assertions)
🇫🇷France duaelfr Montpellier, France

I think I found the issue with my test coverage! Crossing fingers ;)

🇫🇷France duaelfr Montpellier, France

@lendude destination is kept after the redirect. I believe it's been made that way in the original issue.

🇫🇷France duaelfr Montpellier, France

I am not pleased that this checkbox is out of the group.
I'll try to figure out another method to fix the issue.

🇫🇷France duaelfr Montpellier, France

This has been introduced in CKEditor after this module has been created.
I'm glad we don't have to maintain this anymore ;)
Thanks!

🇫🇷France duaelfr Montpellier, France

duaelfr made their first commit to this issue’s fork.

🇫🇷France duaelfr Montpellier, France

@smustgrave I didn't manage to find existing coverage for other allowed tags. Would you help me find it so I can extend it, please?

🇫🇷France duaelfr Montpellier, France

Fix and tests pushed in a MR.
Patch attached for composer. Applies on both 11.x and 10.4.x.

🇫🇷France duaelfr Montpellier, France

Updated IS with references for every tag added to the list.

🇫🇷France duaelfr Montpellier, France

Added to allowed tags in the MR (from 🐛 element stripped by $adminTags Active ).
Patches for composer.

🇫🇷France duaelfr Montpellier, France

Added <template> to allowed list to the MR (from Xss::filterAdmin() to allow "template" elements Active ).
Patches for composer.

🇫🇷France duaelfr Montpellier, France

Here is a simplified patch for Drupal 10.4 for live projects.

🇫🇷France duaelfr Montpellier, France

Fixed missing attributes lost in the reroll.
Added <audio> tag as well. (thanks #86 for that suggestion)
Again, attached patch for composer.

🇫🇷France duaelfr Montpellier, France

Rerolled and cleaned MR.
Added <button> to the allowed list.
Patch attached for composer (11.x).

🇫🇷France duaelfr Montpellier, France

This is quite old!
I was digging into an issue with inline templating within Views and found this piece of code from \Drupal\views\Plugin\views\PluginBase::viewsTokenReplace():

      // Use the unfiltered text for the Twig template, then filter the output.
      // Otherwise, Xss::filterAdmin could remove valid Twig syntax before the
      // template is parsed.

      $build = [
        '#type' => 'inline_template',
        '#template' => $text,
        '#context' => $twig_tokens,
        '#post_render' => [
          function ($children, $elements) {
            return Xss::filterAdmin($children);
          },
        ],
      ];

      // Currently you cannot attach assets to tokens with
      // Renderer::renderInIsolation(). This may be unnecessarily limiting. Consider
      // using Renderer::executeInRenderContext() instead.
      // @todo https://www.drupal.org/node/2566621
      return (string) $this->getRenderer()->renderInIsolation($build);

Here, even if we are using Twig's render engine, filterAdmin is called on post_process without any way to bypass it. In my case, I have an inline template that includes an image field rendered with a responsive style. The <picture> and <srcset> tags are dropped by this post process so I must use an external template instead of the builtin feature of views.

🇫🇷France duaelfr Montpellier, France

@maintainers: Hi! I'm willing to provide a fix but I'd need a bit of guidance because I can't figure out where this is happening. Would you help me, please?

🇫🇷France duaelfr Montpellier, France

I can confirm this issue (on a website without layout_paragraphs, cf #4).
Attaching a screencast of the bug.

As the issue can lead to unnoticed data corruption, I'm bumping it to Major priority.

🇫🇷France duaelfr Montpellier, France

Hi there!
Thank you for working on this issue.
Beside @jonathanshaw comments on the MR, this needs to be rerolled now a few changes has landed.
Cheers!

🇫🇷France duaelfr Montpellier, France

This issue is worked on in Add support for media reference fields Needs review

🇫🇷France duaelfr Montpellier, France

Thank you for suggesting that change!

🇫🇷France duaelfr Montpellier, France

Thank you all for your help with this!

Production build 0.71.5 2024