πŸ‡­πŸ‡ΊHungary @nagy.balint

Account created on 13 January 2012, about 12 years ago
#

Recent comments

πŸ‡­πŸ‡ΊHungary nagy.balint

I created πŸ› TypeError in admin/content/site-settings Fixed for that purpose.

πŸ‡­πŸ‡ΊHungary nagy.balint

Yes currently 1.x is broken due to the commit here since "use Drupal\Core\Session\AccountInterface;" was removed.
This caused admin/content/site-settings to throw a TypeError

It might be easier to just open a new issue, and push this 1 line commit, unless the 1.x branch is discontinued soon.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

Yes, unfortunately I cannot reproduce it either.

Could be something related to server configuration, or collision with another module.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

It seems the issue is even bigger since 10.2 because the ListItemBase.php is changed now and it has code parts like
core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php

    if (!$form_state->get('items_count')) {
      $form_state->set('items_count', max(count($allowed_values), 0));
    }

must be of type Countable|array, null given in count()
or

$current_keys = array_keys($allowed_values);

must be of type array, null given in array_keys()

Because it is possible to empty the allowed values by manually editing and importing config, but since 10.2 the system fails when the $allowed_values is empty.

πŸ‡­πŸ‡ΊHungary nagy.balint

Works for me as well on Drupal 10.2.2 and tablefield 2.4, php 8.2

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

Are you using the JJJ fork of chosen?

I see this commit https://github.com/JJJ/chosen/pull/15 but then I guess that was not enough to fix the issue?

πŸ‡­πŸ‡ΊHungary nagy.balint

I think, this is a very important project, and eventually we will have issues if it is left without maintenance.

As the last reply is 7 months ago, I am not sure if we shouldnt try the procedure outlined at the following link:
How to become project owner, maintainer, or co-maintainer β†’

@Anybody, what do you think?

(based on the issue queue I set higher priority)

πŸ‡­πŸ‡ΊHungary nagy.balint

Are you absolutely sure that you are using the latest 2.0.9 version and cleared the drupal cache?

πŸ‡­πŸ‡ΊHungary nagy.balint

Can you try the latest version? There was an issue with saving the widget.

πŸ‡­πŸ‡ΊHungary nagy.balint

I reopen this, as I found an issue when saving the widget settings with empty day select.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

Try to keep the date format at the default.

To change the date format displayed to the user, the Aleternative Format should be used. (See image)

πŸ‡­πŸ‡ΊHungary nagy.balint

I think it is better to keep them separate.

This option could be implemented similarly to "Ignore logging on config import" checkbox.

So we could have a checkbox in the settings like "Omit logs with no modifications", and then the filter method could be in the base class, and each subscriber could call it to filter the logs.

πŸ‡­πŸ‡ΊHungary nagy.balint

As this is only needed for 2 subscribers out of the 3, for now I just added it to the other one, and also fixed the code style.

πŸ‡­πŸ‡ΊHungary nagy.balint

So then, this can be closed as the issue is with the other patch that needs a reroll.

πŸ‡­πŸ‡ΊHungary nagy.balint

Fixed a deprecation warning on php 8.1.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

It is good, but similarly to πŸ› Fix "Ignore logging on config import" feature Needs work , we should always consider that there are other subscribers, and this could be a more global change.
For example sending a mail about a no change could be useless as well.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

There are some sites that use the 3.x branch and would like to move to Drupal 10.
As 4.x is a rewrite it seems, I attach a patch for the 3.x branch for compatibility.

If the maintainers would like to make it easier for sites that run 3.x to keep using their existing module in Drupal 10, then it can be committed to the 3.x branch.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

1. Normally if I search for a date I would search for a specific date like "2023-10-12" or in between.
in between works, though we need to put 2023-10-11 and 2023-10-13 to search for entries 2023-10-12.
but equal to does not work for me. So currently I cant list the items with date of 2023-10-12 unless I use the in between above.

2. The config_log_views module should depend on config_log, so when config_log is uninstalled we also uninstall the submodule.

(And a followup issue could be that normally we should not log config changes that are "no change", for example i got like 5 lines with no change on language.types. But that is unrelated to this issue.)

πŸ‡­πŸ‡ΊHungary nagy.balint

I think it is fine, as this is an administrative view anyways.

πŸ‡­πŸ‡ΊHungary nagy.balint

Collapsible is also good.

Could we add some basic date filter? So the entries on a specific day can be found more easily.

πŸ‡­πŸ‡ΊHungary nagy.balint

Very nice!

I am afraid however, that this will require a bit more work.

1. We will need a fallback for old entries where the original data does not exist, otherwise we get
Deprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Drupal\config_log_views\Plugin\views\field\ConfigLogDiff->render() (line 69 of modules/contrib/config_log/modules/config_log_views/src/Plugin/views/field/ConfigLogDiff.php).

2. If the diff is big, then the display becomes problematic, especially as the other columns put the text in the vertical center, so the user basically cant even see what the diff is about. I guess that is why when the admin/config/development/configuration page displays diffs, it is always done in modals, and not directly on the page.
Or if we can do it selectively and only put the big diffs into a modal would be even better.
Not sure what we can do to improve the situation without causing a lot of extra work, but we will need to do some improvement, as it is likely to have some big diffs I think.

πŸ‡­πŸ‡ΊHungary nagy.balint

Unfortunately this does not seem to work

>  [notice] Update started: config_log_views_update_8001
>  [error]  You have requested a non-existent service "config_update.config_update".
>  [error]  Update failed: config_log_views_update_8001
πŸ‡­πŸ‡ΊHungary nagy.balint

I think we need a new update hook, as otherwise sites that already run this update hook won't run this new part.

πŸ‡­πŸ‡ΊHungary nagy.balint

Looks nice!

Not sure if we need some basic date filter.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

I think it is a good idea to include that.

I wonder how the diff will look in that view.

πŸ‡­πŸ‡ΊHungary nagy.balint

4.x version updates.

πŸ‡­πŸ‡ΊHungary nagy.balint

Updates for the 4.x version.

πŸ‡­πŸ‡ΊHungary nagy.balint

Added ddev instructions for install.

πŸ‡­πŸ‡ΊHungary nagy.balint

Change for the 4.x version.

πŸ‡­πŸ‡ΊHungary nagy.balint

Update for ddev informations.

πŸ‡­πŸ‡ΊHungary nagy.balint

Thanks!

I think the patch is still needs work, as Database subscriber also has the import event.

πŸ‡­πŸ‡ΊHungary nagy.balint

It is good, but on a fresh install with simlytestme I still got Tag based cache for some reason

πŸ‡­πŸ‡ΊHungary nagy.balint

Thanks!

I just realised that this views config however won't be imported on existing installs of the module.

So we have two ways to go:
a) We can create a sub module (something like config_log_views) and then anyone can enable the new sub module to get the views integration.

b) We can create an update hook to read this config and apply it.

Maybe a) is better, as even though db table logging is the default setting, it can be configured to not log anything into the database, so in some configurations this views integration is not needed, or maybe the developer does not need it.

πŸ‡­πŸ‡ΊHungary nagy.balint

Thank you!

1. However the UUID should not be part of the yml file: https://www.drupal.org/docs/creating-modules/include-default-configurati... β†’

https://www.drupal.org/project/config_devel β†’ could help as well.

2. The Tag based caching should be disabled on the view, cause otherwise it will not show the recent entries, or only after cache clear.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

A few things were removed, and the starter kit was converted to layout paragraphs.
I will do some edits there soon.

Let me know if you have questions, then I can add it to the documentation.

πŸ‡­πŸ‡ΊHungary nagy.balint

Then we can have 3 options in the readme.

1. merge plugin - if possible, is the better approach as then it takes the version defined in the module.

but it is sometimes not possible or problematic so the other two options:
2. defining the repository as #7
3. defining the installer-paths as #11

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

I would say this is more a critical issue, since it can break the site.

Thanks for the patch!

πŸ‡­πŸ‡ΊHungary nagy.balint

So basically what we can add to the README is the following:

Add the following entry to the root composer.json inside the "repositories" section:

        {
            "type": "package",
            "package": {
                "name": "jjj/chosen",
                "version": "2.2.1",
                "type": "drupal-library",
                "source": {
                    "url": "https://github.com/JJJ/chosen.git",
                    "type": "git",
                    "reference": "2.2.1"
                }
            }
        },

Then you can run the following command to install the library to the right folder:
composer require jjj/chosen:2.2.1

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

Thanks!

1)
If I understand correctly this can happen if a layout was removed while the module's config still had data for it, and so the update can fail in that case.
Although won't this leave the incorrect config in the module's config intact then?
As the update hooks change structure, but they don't completely replace the full array without the skipped items.

2)
Couldn't we use something like

if (!($layoutDefinitions[$lid] instanceof LayoutDefinition)) {
        continue;
    }

So the patch is easier to review, and -> https://www.youtube.com/shorts/Zmx0Ou5TNJs

πŸ‡­πŸ‡ΊHungary nagy.balint

I guess for something like this to get into the module,
we would need a UI that is similar to what is in metatags module for example, where one can create config sets, and either create a global one, or one for a specific content type that takes precedent.

πŸ‡­πŸ‡ΊHungary nagy.balint

I guess the use case here could be if we would like to limit the layouts differently for each content type.

So for content type A, and layout X we would like to allow a type, while not allowing it for layout Y,
but for content type B, and layout X we would not like the allow that type, and lets say allowing it for layout Y.

Then of course a further layer of config would be necessary, however I'm not sure if that would not make the config a bit too difficult to manage.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

As far as I know it is possible at the field setting of the paragraph field to set which paragraphs types are available.
Which even works when reusing the field.

While this module allows to restrict further by setting up which paragraphs types (from the allowed ones) are available in each layout or region of the layout.

So for now I am not sure we should change the config, as it is already possible to do, unless I'm missing something.

πŸ‡­πŸ‡ΊHungary nagy.balint

With the following patch it seems to work fine for me.
I set the accesscheck to false, as anyways this batch should process all nodes I suppose.

And the batch start does not need the settings parameter it seems,

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

Thanks!

I would say however that we should have both in the Readme.

As far as I understand the path repository is a manual process, and it was always possible in the past to do that manually.
But the disadvantage is that if the module changes the dependency, then the user might not update his own composer json manually, so that could be an issue.

While the merge plugin solved that issue by including the composer json from the module.

I think the deprecation happened when the merge plugin was not maintained temporarily, but now it is actively maintained actually.

But I think we should have this other option in the Readme anyways, because if merge plugin cannot be used on Pantheon, or there are other cases where it's not possible to be used then there should be this other way of managing it.

πŸ‡­πŸ‡ΊHungary nagy.balint

Here is a quickfix for the problem.

Of course this will not make it work when facets are present on the view.

But it will fix normal views like the media library which normally has no facets but has to work with ajax most of the time.

πŸ‡­πŸ‡ΊHungary nagy.balint

Thank you for the patch!
It works for us so far.

πŸ‡­πŸ‡ΊHungary nagy.balint

I also needed this patch.

It seems to work fine!

πŸ‡­πŸ‡ΊHungary nagy.balint

Reverted it for now, as composer does not put back the version number unless I manually delete the module.

So then this could be a big issue for anyone on dev version.

πŸ‡­πŸ‡ΊHungary nagy.balint

So this dependency does not work with dev versions even if they are 2.0.x.

So we either need a way to fix that issue, or we need to revert this unfortunately.

Since for dev version of layout paragraphs, this will block the update.php even.

πŸ‡­πŸ‡ΊHungary nagy.balint

Or we can add an extra check in js.

πŸ‡­πŸ‡ΊHungary nagy.balint

Unfortunately the separate input widget does not work properly when the field is kept empty, as the validation fails.
Also I think it should not check with >= as then when the date is the exact same it will throw a validation error.

πŸ‡­πŸ‡ΊHungary nagy.balint

Thanks!
It works for me.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

This part of the code

if (empty($disallowed_types[$parent_layout][$region]['paragraph_types'])) {
      return;
    }

Was done cause of https://www.youtube.com/shorts/Zmx0Ou5TNJs

Apart from that, maybe we just need to explain this limitation with duplicate on the UI somewhere, and we can still have this feature?

πŸ‡­πŸ‡ΊHungary nagy.balint

The DateTimeFlatPickrWidgetTrait already existed, so this MR is quite incorrect.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

Nice!

However I do not see the changes yet on the javascript file, which prevents moving the paragraph to the incorrect region.

πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

I think that potentially the DateTimeFlatPickrWidgetTrait could be used here to avoid code duplication.

If needed the trait can be changed slightly to make that possible, like if the public static function defaultSettings() { needs to be separated into two methods (or that the trait only provides the default settings array, and the + parent::defaultSettings(); could be in the specific classes).

πŸ‡­πŸ‡ΊHungary nagy.balint

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

πŸ‡­πŸ‡ΊHungary nagy.balint

Thanks!

I have the same feedback here as on πŸ“Œ Update description on settings page Needs work

also likely it should still be called Configuration instead of Getting Started according to https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... β†’ README sections

πŸ‡­πŸ‡ΊHungary nagy.balint

Thanks!

I think that the Note is not clear enough, and should be something like:

Only the layouts that have been enabled in the settings of the 'Layout Paragraphs' behavior will be shown here.

πŸ‡­πŸ‡ΊHungary nagy.balint

So likely as $disallowed_types[$parent_layout][$region] can be empty array we should have
if (empty($disallowed_types[$parent_layout][$region]))

Otherwise this should be ready to go.

πŸ‡­πŸ‡ΊHungary nagy.balint

Nice!

However I think it reverts a previous patch at

 // If no disallowed types, return.
-    if (empty($disallowed_types[$parent_layout])) {
+    if (!isset($disallowed_types[$parent_layout][$region])) {
      return;
    }
πŸ‡­πŸ‡ΊHungary nagy.balint

Hi!

This patch is unfortunately not created based on the dev version.

πŸ‡­πŸ‡ΊHungary nagy.balint

The module loads the js library as

 js:
    https://unpkg.com/micromodal@0.4.10/dist/micromodal.min.js: { type: external, minified: true }

in media_video_micromodal.libraries.yml

This means that it simply pulls the lib from external source.

This also means that if I would like to make a contrib module that also works with this micromodal, but for image media,
then I also need to implement a libraries yml and pull the library as an external source.

In case I use both modules, then it will potentially pull it in twice, but at least when the version changes in one of the modules, will certainly pull it in twice.

The solution could be to have a micromodal contrib module which the other modules depend on, then we can avoid that problem.

Or to have a solution where we can include the library through composer, with for example the merge plugin. But in the current situation even if the other module used composer, this module would still pull the library in as an external.

Production build https://api.contrib.social 0.61.6-2-g546bc20