Hi Yannick,
Thanks for this issue. I agree about letting the site builder input the key even if I'm not pleased by that.
Could you please keep your MR focused on this scope and open other issues for all the other changes you would like to see in the module?
I know it's not fun but it would make my reviews easier and provide you more credits ;)
Cheers!
Thank you for reporting this issue!
I merged most changes in the code for the upcoming release.
I heard what you said about the error not being clearly announced to screenreaders but the thing is that it's a Core design issue. The inline form errors pattern has been adopted in core since Drupal 8 → . It's supposed to be more accessible than having all errors directly in the messages section. I'm not an accessibility expert in any way but I've had to deal with some accessibility audits in the past that reported problems about other things related to form errors but not the way errors were presented to the user. For now, I decided to stick to the Core way of doing this, and thus, to properly display the full error under the field.
I believe it's still better than before for accessibility and I hope it will be better with Core continuing to work on the subject.
I'm sorry it took so long.
Thanks for the reminder!
Thanks!
This is fixed and will be included in the upcoming 2.2.10 and 2.3.2 versions.
The MR contains a lot of unrelated minor changes. I believe we should commit them separately to make it easier to review this massive MR and focus on the actual changes.
2.2.x is green! Now let's fix 2.3.x... but another day for me ;)
This has been added a long time ago. I updated the content to ensure labels were up to date.
Will be included in the upcoming 2.2.9 release.
Thanks for reporting. This is now fixed and will be included in the upcoming 2.2.9 release.
If someone else is looking for this, we can now use:
CORE_SECURITY_PREVIOUS_MINORforcomposer (previous minor)CORE_PREVIOUS_STABLEforcomposer (previous major)CORE_NEXT_MINORforcomposer (next minor)CORE_MAJOR_DEVELOPMENTforcomposer (next major)
See https://git.drupalcode.org/project/gitlab_templates/-/commit/2752d072012...
Rerolled MR on last 11.x.
Made a minor change to help applying this with other patches.
Providing patches for 11.x and 10.5.x for composer.
Rerolled on last 11.x.
Added missing config schema entry for the new setting.
Patch attached for composer lovers ;)
Yay! All green!
Rerolled and resolved remaining comments
Rerolled #84 on the last 11.x commit in a MR.
Here is the patch for composer.
This issue still needs a Change Record. Is there a native english speaker that could handle this?
More progress today:
* Rebased our branch on latest 2.0.x commit
* Factorize views_styles slots conversion
* Factorize and extend dependencies conversion
* Fix 1.x missing dependencies during dependencies conversion
* Fix field_formatters variants_token conversion
* Document and harmonize views style settings conversion
* Add field_group settings conversion
* Add views row settings conversion
To be continued
A bit of progress today:
* Rebased our branch on latest 2.0.x commit
* Started to factorize settings to props conversion
* Prepared future conversion of mappings to slots
* Wrote the field_formatters settings conversion
To be continued.
The Attribute object already filters HTML from values so that shouldn't be an issue. I believe this is covered by some tests.
I'm on it in 2.2.x branch then I'll reproduce in 2.3.x
vladimiraus → credited duaelfr → .
Hi!
Can you try with the latest 2.3.x dev version please?
I pushed some cleanup that, I think, should fix this issue.
Something went wrong in my previous rebase. Here is the good one and the attached patch file.
I just rebased the MR so it can easily be applied on the dev branch.
Here is the patch for composer users that would need this fix right now :)
I would need that!
My use case: my client asks me to enable filename sanitization for new uploaded files and to sanitize our 50K+ existing files with redirects to keep the SEO juice.
I tried to manually create a Redirect from /sites/default/files/MYFILE.pdf to /sites/default/files/myfile.pdf but when trying to access /sites/default/files/MYFILE.pdf, I get a 503 error because of the RedirectLoopException.
The unlink button has been added in
📌
Improve usability
Active
.
Sorry it took so long.
Thank you for your work here. As told in
🐛
Drupal 10.5 support - replace usages of .once
Active
, the issue was not about the once function so I'm marking this as a duplicate.
Thank you all for your work here!
The issue is not really related to jquery.once because the once functions used by the modules are those provided by CKEditor.
The issue was that CKEditor made a breaking change in their internal. See
🐛
Update drupalFile.js for compatibility with the latest CKEditor version
Active
.
Thank you all for fixing this!
@julien tekrane Would you provide me your patterns in an attached zip file so I can try to reproduce locally?
@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?
Good point. Thanks!
duaelfr → made their first commit to this issue’s fork.
@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?
@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.
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 theisFieldVisible()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.
@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?
phpstan should be fixed now but I'm not sure it's the best way...
May I have an ultimate review on this?
@pdureau I rebased the MR on the latest commit.
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).
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.
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.
- I installed Drupal 10.3.14 on simplytest.me
- I configured the "Basic HTML" text filter to use this module's plugin (but not the "Full HTML" filter)
- I copied a bunch of content from https://en.wikipedia.org/wiki/Drupal
- I pasted it into my editor in "Full HTML" format, markup was preserved
- 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)
Existing tests are stable. Let's merge this before moving on to the next major.
Existing tests are stable. Let's merge this before moving on to the next major.
Converted the patch in a MR to let the CI do its magic
Existing tests are stable. Let's merge this before moving on to the next major.
Existing tests are stable. Let's merge this before moving on to the next major.
This has been fixed in
📌
[1.11.0] Fix CI failures
Active
to get tests green.
Thanks!
This one made my cry blood. I hope next ones will be easier ;)
(Sorry for the above commits. It was a rebase mistake.)
Done (no need for a new patch as it's only a typo in a comment).
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.
There is a proposal.
Patch attached for composer.
New attempt to fix the issue.
Patch for composer attached.
@pdureau Thanks for the review.
Do you have some examples of other locations where patterns can be found? How can this be dynamically determined?
Done
Patch attached for composer
There you go.
Patch attached for composer users.
FYI, the Audit Files → module provides a report that tries to match existing duplicated files and allow to merge them.
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.patchon the ckeditor module2741945-103-core-10.4.patchon Drupal Core (tested on 10.4 but might also work on previous 10.x versions)
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.
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.