Porta Westfalica
Account created on 9 May 2008, over 17 years ago
  • Drupal Software Engineer & Developer, Project Management at DROWL.de 
#

Merge Requests

More

Recent comments

🇩🇪Germany Anybody Porta Westfalica

Thank you very much @pookmish! :)
Maybe you have some time to also look into the other issues?

Otherwise my offer still stands: 💬 Offering to co-maintain Asset Injector Postponed

🇩🇪Germany Anybody Porta Westfalica

@patrick r. or someone else, could you please turn the better and working solution into a MR to proceed here?

🇩🇪Germany Anybody Porta Westfalica

Totally makes sense!

🇩🇪Germany Anybody Porta Westfalica

Well I think the root cause has to be investigated... this seems a step too late?

🇩🇪Germany Anybody Porta Westfalica

@blacksnipe could you or someone else please prepare the fix as MR?

🇩🇪Germany Anybody Porta Westfalica

Furthermore the limit should be configurable for the user with a useful default, eg. 10?

🇩🇪Germany Anybody Porta Westfalica

@fatima2345 please be more specific. What happens and what is expected?

🇩🇪Germany Anybody Porta Westfalica

Yup, +1! This is a bit confusing otherwise!

🇩🇪Germany Anybody Porta Westfalica

@bgilhome really nice work and cleanup! Could you maybe turn this into a MR instead of a patch to proceed more easily?

🇩🇪Germany Anybody Porta Westfalica

@rrotari great idea! That totally makes sense and would be helpful. Could you or someone else please turn this into a MR instead of using a patch?

🇩🇪Germany Anybody Porta Westfalica

Great work @grevil! Please see the comments. Everything else looks great and this is a really nice feature!

🇩🇪Germany Anybody Porta Westfalica

I think this is a good task for beginners?

🇩🇪Germany Anybody Porta Westfalica

Still needs tests ad there's an unresolved comment.

🇩🇪Germany Anybody Porta Westfalica

@joevagyok please test and review! :)

🇩🇪Germany Anybody Porta Westfalica

Thanks @dobe that's indeed a good point I didn't think about.

🇩🇪Germany Anybody Porta Westfalica

@dobe please use MRs instead of patches. Your patch is correct, thank you for the fix! :)

🇩🇪Germany Anybody Porta Westfalica

@dobe please use MRs not patches.

@LR: Could you please have a look?

🇩🇪Germany Anybody Porta Westfalica

Thanks @goodboy - technically that would mean that these modules need to switch from ban to advban in their .info.yml right? Or could advban work on top of ban in the future, "hot-swapping" / overriding ban without the need to remove the ban dependency in the contrib modules? (That would be fabulous architecturally!)

🇩🇪Germany Anybody Porta Westfalica

Thanks @welly! media_duplicates also shows such a warning on media upload and even prevents the upload IMHO.

🇩🇪Germany Anybody Porta Westfalica

Also see #6 and #8 in 💬 Can't find the right token (URL) for og:image in Drupal 10 Active . I think the very special behaviour leads to unexpected results for people used to tokens. Especially as the same token behaves differently everywhere else.

Maybe a better solution would be to provide more powerful tokens so that the special behaviour can be removed here?

🇩🇪Germany Anybody Porta Westfalica

I can also confirm we are running into the (unexpected) behaviour described in #6

Using :url ([node:field_mediaref_on_node:entity:field_media_image:url]) "works", but the resulting URL is relative (starting with a /!) while I think an absolute URL would be expected.

Using :url:absolute does not work at all (empty).

I think the very special behaviour of this field needs to be pointed out and maybe it should be improved or removed. Unsure if More helpful description for the og:image field Active is enough.

Using an image style indeed leads to the expected result as absolute URL! (Without ending :url)

🇩🇪Germany Anybody Porta Westfalica

Thanks @lrwebks I think it should be similar to how translation of field formatters and field widgets work?

At least the schema structure looks similar:

entity_extra_field.extra_field.*.*.*:

But I have to admit that I also don't know how to implement that correctly, yet. Let's see if someone know the answer!

config changes already look good!

🇩🇪Germany Anybody Porta Westfalica

@Maintainers: Any feedback so we can finish this finally? :)

🇩🇪Germany Anybody Porta Westfalica

@Maintainers: Any chance to merge this finally?

🇩🇪Germany Anybody Porta Westfalica

@jsacksick I'm totally fine with both, just think it should be possible. Which way should we go?

🇩🇪Germany Anybody Porta Westfalica

@silvi.addweb please use a MR instead

🇩🇪Germany Anybody Porta Westfalica

Not unsetting '_actions' and 'weight' in the callback, seems to fixed it without any seeming regression.

This was introduced in #31 FYI: https://www.drupal.org/files/issues/2024-10-10/entity_browser-2772279-31... by @gugalamaciek.

The first patch didn't contain that part of the code.

🇩🇪Germany Anybody Porta Westfalica

I can now confirm that this caused huge trouble in a production project. Needs to be fixed and released ASAP!

🇩🇪Germany Anybody Porta Westfalica

Sorry, there's already an example for the settings form field in latest dev! :)

🇩🇪Germany Anybody Porta Westfalica

anybody changed the visibility of the branch 3480580-add-settings-defaults-example to hidden.

🇩🇪Germany Anybody Porta Westfalica

Let's also add example values to the form description in the setting!

🇩🇪Germany Anybody Porta Westfalica

@thomas.frobieter @phjou same here, I created a dedicated issue: 📌 Offering to Maintain "PeerTube" module Active

🇩🇪Germany Anybody Porta Westfalica

anybody created an issue.

🇩🇪Germany Anybody Porta Westfalica

Contrib request in advban module: IP Whitelist Active

Still a very valid request!

🇩🇪Germany Anybody Porta Westfalica

anybody created an issue.

🇩🇪Germany Anybody Porta Westfalica

@thomas.frobieter please test if this works.

🇩🇪Germany Anybody Porta Westfalica

The once functions take 2 parameters, a string id without spaces and an array-like object. The recommended way to use it is to call it with the string you would use in a querySelectorAll() call. You can only use once with objects that are instances of Element, which means you can not use it with the document or window object. If you need a page-level element please use 'html' or 'body' as the selector used in the once call: once('my-global-once', 'html').

https://www.drupal.org/node/3158256

🇩🇪Germany Anybody Porta Westfalica

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

🇩🇪Germany Anybody Porta Westfalica

Thanks @pcambra, same here!

With https://www.drupal.org/project/extra_field_plus you add another important point for using a class:
Allow to add settings for extra fields (e.g. widget / display settings). Guess that should be handled by (inherited) methods.

As @mxh wrote in #50 we should elaborate, where extra fields are placed and how they may interact with widgets and formatters. Maybe that also solves the settings partially. I agree extra fields might be a (yet missing) intermediate layer between the storage and the formatters / widgets.

🇩🇪Germany Anybody Porta Westfalica

@mxh nice draft! Maybe we should put that into a separate issue then?

Some additional thoughts on the draft:
1. entity_type(s) parameter:
#[ExtraViewField(id: 'my_extra_field', entity_type: 'node')]
Would it make sense to allow more than one entity type:
#[ExtraViewField(id: 'my_extra_field', entity_types: ['node', 'user'])]

Or even leave out that parameter to have it on all existing entities automatically?
#[ExtraViewField(id: 'my_extra_field')]

2. Optional display parameter:
#[ExtraViewField(id: 'my_extra_field', entity_type: 'node', displays: ['teaser'])]

3. Configure field ui visibility:
Hide the field entirely in the field ui sorting, only make it available programmatically
#[ExtraViewField(id: 'my_extra_field', ..., visible: FALSE]

4. Set "disabled" by default (maybe this should be assumed):
#[ExtraViewField(id: 'my_extra_field', ..., disabled: true]
🐛 Show extra fields (hook_entity_extra_field_info) only in "Default" display mode by default Active
or
#[ExtraViewField(id: 'my_extra_field', ..., region: 'disabled']
So that the field in region "disabled" by default.

Just some ideas to discuss, which I had in the past regarding extra fields... Maybe some of them could alternatively better be methods of the interface?

Another option would be to annotate functions, not classes, like OOP hooks do ...?! Or maybe better not...
When using a class, the benefit might be to combine the form display, view display and eventually some kind of data handling in one class?!

🇩🇪Germany Anybody Porta Westfalica

@nishantghetiya thanks, nice!!

  • The bundles should be stored as sequence, not as string with separator
  • This should also be implemented for the other entity types
🇩🇪Germany Anybody Porta Westfalica

If anyone else encounters this: My workaround now is to use https://www.drupal.org/project/views_raw_sql and format the date field with custom SQL and hide the original date field. Afterwards, the output can be used for grouping.

🇩🇪Germany Anybody Porta Westfalica

I created a bug report, because this module currently only seems to work with "timestamp" date fields and none of the other date fields from Drupal core: 🐛 Only works with "timestamp", not "date" or "datetime" Active

I think this issue is related, even if it talks about "custom date fields"

🇩🇪Germany Anybody Porta Westfalica

Yes, maybe my thoughts were just too complicated, I thought maybe others could also ask themselves if and how these replacements work for OOP hooks. So I think an example wouldn't hurt, but I'm also fine with not adding that.

🇩🇪Germany Anybody Porta Westfalica

Here's the related change record: https://www.drupal.org/node/3493962

🇩🇪Germany Anybody Porta Westfalica

@bluegeek9 see https://www.drupal.org/node/3442349 and https://www.drupal.org/node/3485896 . With OOP Hooks ordering is now a lot cleaner and easier, if it helps.

🇩🇪Germany Anybody Porta Westfalica

Smart date support would really be awesome! @wendyZ @R_H-L could you maybe prepare this as MR instead of a patch to speed things up?

🇩🇪Germany Anybody Porta Westfalica

Sorry @lazzyvn you're right, that was indeed too quick.

🇩🇪Germany Anybody Porta Westfalica

@mxh sorry my fault, indeed I now remember I saw that long ago, and think I also used it in a contrib project, but forgot about it. Instead, I used hook_entity_extra_field_info() a lot in such cases.

Now I'm indeed unsure, if the Plugin idea still makes sense. Let's all think about that for a while. Thank you for the reminder and sorry I didn't remember that.

🇩🇪Germany Anybody Porta Westfalica

This is surely just a feature request.

🇩🇪Germany Anybody Porta Westfalica

Leaving this RTBC because of the missing credit.

🇩🇪Germany Anybody Porta Westfalica

Seems this was once again merged without credit from the maintainer.

🇩🇪Germany Anybody Porta Westfalica

Was merged - with no credits yet.

🇩🇪Germany Anybody Porta Westfalica

@fago yes, kind of. I think the whole thing I described is around computed / extra / dynamic / pseudo fields. My proposal was a more complete out-of-the-box solution for such dynamically calculated / derived fields in core, without storage.
Guess with computed fields you mean the contrib project , that also allows storing these values then. I think that part should be left to contrib indeed.

TL;DR: I was talking about making such dynamic fields a first-class-citizen in core. hook_entity_extra_field_info() doesn't feel like that in contrast to other Plugin concepts. Maybe it could be summed up with sth. like: Improve Extra Fields in core and use Plugins.

Is that a bit clearer?

🇩🇪Germany Anybody Porta Westfalica

Great, all green! :) Let's merge this. Please reopen if further issues with the view should appear.

🇩🇪Germany Anybody Porta Westfalica

@grevil: Please review code-wise.
@lrwebks please test if this works as expected and nothing is broken

🇩🇪Germany Anybody Porta Westfalica

@dobe Please use MR's instead of patches. Furthermore, this is complicated enough to better be a setting to opt-into!
Please separate the code as much as possible.

🇩🇪Germany Anybody Porta Westfalica

@grevil: GREAT! I set this fixed, but didn't merge ... -.-

Production build 0.71.5 2024