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
@patrick r. or someone else, could you please turn the better and working solution into a MR to proceed here?
Totally makes sense!
@maxilein any ideas how this can happen?
Well I think the root cause has to be investigated... this seems a step too late?
@blacksnipe could you or someone else please prepare the fix as MR?
I think this is major.
Furthermore the limit should be configurable for the user with a useful default, eg. 10?
One should think && !$user->user_picture->isEmpty()
is enough, but it isn't, we ran into this several times...
anybody → made their first commit to this issue’s fork.
@fatima2345 please be more specific. What happens and what is expected?
Yup, +1! This is a bit confusing otherwise!
Nice work, RTBC! :)
@bgilhome really nice work and cleanup! Could you maybe turn this into a MR instead of a patch to proceed more easily?
@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?
Great work @grevil! Please see the comments. Everything else looks great and this is a really nice feature!
I think this is a good task for beginners?
Still needs tests ad there's an unresolved comment.
@joevagyok please test and review! :)
Thanks @dobe that's indeed a good point I didn't think about.
@dobe please use MRs instead of patches. Your patch is correct, thank you for the fix! :)
@dobe please use MRs not patches.
@LR: Could you please have a look?
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!)
Thanks @welly! media_duplicates also shows such a warning on media upload and even prevents the upload IMHO.
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?
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
)
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!
@Maintainers: Any feedback so we can finish this finally? :)
@Maintainers: Any chance to merge this finally?
@jsacksick I'm totally fine with both, just think it should be possible. Which way should we go?
@silvi.addweb please use a MR instead
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.
I can now confirm that this caused huge trouble in a production project. Needs to be fixed and released ASAP!
Thanks @grevil!
Sorry, there's already an example for the settings form field in latest dev! :)
anybody → changed the visibility of the branch 3480580-add-settings-defaults-example to hidden.
Let's also add example values to the form description in the setting!
@thomas.frobieter @phjou same here, I created a dedicated issue: 📌 Offering to Maintain "PeerTube" module Active
anybody → created an issue.
Contrib request in advban module: ✨ IP Whitelist Active
Still a very valid request!
@thomas.frobieter please test if this works.
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').
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.
@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?!
@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
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.
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"
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.
Here's the related change record: https://www.drupal.org/node/3493962 →
@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.
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?
Sorry @lazzyvn you're right, that was indeed too quick.
@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.
This is surely just a feature request.
Leaving this RTBC because of the missing credit.
Seems this was once again merged without credit from the maintainer.
Was merged - with no credits yet.
@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?
Great, all green! :) Let's merge this. Please reopen if further issues with the view should appear.
@grevil: Please review code-wise.
@lrwebks please test if this works as expected and nothing is broken
@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.
@grevil: GREAT! I set this fixed, but didn't merge ... -.-
Ok let's see if this works. Looking good locally.