Strasbourg
Account created on 26 October 2018, over 5 years ago
#

Merge Requests

More

Recent comments

🇫🇷France NicolasGraph Strasbourg

Sorry, I was wrong... I mean, the following line set the time to midnight anyway : $prior_period->setTime(0, 0, 0);.

🇫🇷France NicolasGraph Strasbourg

As there is no native way to check for the existence of an item into a queue and as the payment entity has no data field to flag it as enqueued, let reuse the guest_suite module solution by waiting the queue to be empty before to add new items in it. I don't want to introduce a dependency to queue_unique for now.

🇫🇷France NicolasGraph Strasbourg

Commited. Thank you both!

🇫🇷France NicolasGraph Strasbourg

Thanks @solene_ggd!

🇫🇷France NicolasGraph Strasbourg

Thanks @solene_ggd; I like this approach.

🇫🇷France NicolasGraph Strasbourg

Thanks @sarwan_verma,
Please use the PR I completed to test and review.

🇫🇷France NicolasGraph Strasbourg

Here is a better approach, updating just the modified field params.

🇫🇷France NicolasGraph Strasbourg

Here is the patch to review.

🇫🇷France NicolasGraph Strasbourg

NicolasGraph changed the visibility of the branch 1.x to hidden.

🇫🇷France NicolasGraph Strasbourg

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

🇫🇷France NicolasGraph Strasbourg

Patch #86 causes malformed UTF-8 characters for emojis.

🇫🇷France NicolasGraph Strasbourg

NicolasGraph changed the visibility of the branch 1.x to hidden.

🇫🇷France NicolasGraph Strasbourg

NicolasGraph changed the visibility of the branch 1.x to hidden.

🇫🇷France NicolasGraph Strasbourg

Added screenshots to show changes.

🇫🇷France NicolasGraph Strasbourg

Adding #3419434 📌 Enable Gin edit form for Commerce orders Needs review to the subissues.

🇫🇷France NicolasGraph Strasbourg

The only thing that is kind of weird is the blank sidebar on product variations, but I guess we can't do much here about that. Maybe an issue could be created in commerce to add some more informations there.

🇫🇷France NicolasGraph Strasbourg

Here is a try to get some more enhancements to the order layout.

🇫🇷France NicolasGraph Strasbourg

Thanks for your reply and your work @jsacksick.

🇫🇷France NicolasGraph Strasbourg

Here are some screenshots without the patch using, and with the patch using Claro and Gin.

🇫🇷France NicolasGraph Strasbourg

Using the current theme is actually not a good idea as Gin use the claro classes.

🇫🇷France NicolasGraph Strasbourg

It is missing some more markup and the current theme name is not passed to commerce-order--admin.html.twig.

🇫🇷France NicolasGraph Strasbourg

@amourow, I can't find batch processes running twice on 1.0.x-dev.
Can you give it a try?

🇫🇷France NicolasGraph Strasbourg

Hi @amourow, I think the queue worker and the typo fix in the .install should not be in the current merge request as it is not related to the issue. There was an interesting talk from @xjm on code review and good contributing practices in DrupalCon Lille; slides are available here : https://drive.google.com/file/d/1o6wIX75wXI_XE80NFj_QRJwkM578sFnS/view.
I also suggest you take a look at the tmgmt_deepl module and the way the queue worker is handled as for now you do not create queue items at all as far as I can see. And the module also use the batch API.

🇫🇷France NicolasGraph Strasbourg

I guess we could even close it as a duplicate of https://www.drupal.org/project/tmgmt/issues/3405309 🐛 Computed fields should not be embeddable fields or source suggestions Needs review if nobody have objection.

🇫🇷France NicolasGraph Strasbourg

Moves to "Needs work" as it would require a test.

🇫🇷France NicolasGraph Strasbourg

I think the metatag_computed field should not even be translatable as it is set as computed.
Here is a patch setting computed field as not translatable in ContentEntitySource::extractTranslatableData().

🇫🇷France NicolasGraph Strasbourg

I'm completing the patch using the new delta in setTranslations().
However, @berdir is right about wondering about active jobs with the old structure. It is not supported here.

🇫🇷France NicolasGraph Strasbourg

I'm completing the previous patch to fix the following error on job items pages:
Notice: unserialize(): Error at offset 0 of 94 bytes in Drupal\tmgmt_content\MetatagsFieldProcessor->extractTranslatableData() (line 22 of modules/contrib/tmgmt/sources/content/src/MetatagsFieldProcessor.php).

🇫🇷France NicolasGraph Strasbourg

I guess the settings form would need a rework at some point. The MR just extend the current behaviour/code.

🇫🇷France NicolasGraph Strasbourg

I had an "INVALID FILE NAME SUGGESTIONS" error in the Twig debug comments because of the missing hook prefix in suggestions.
I came up with a quite similar fix but setting the prefix the following way :

$template_id = $variables['content']['#layout']->getThemeHook()
🇫🇷France NicolasGraph Strasbourg

Thank you all for your work on this issue! It does work as far as I tested.
However, Did you think about fixing the backend part directly and only in GinSettings to keep things simple and tidy?
I don't want to pollute current patches but here is what seems to work for me.

diff --git a/src/GinSettings.php b/src/GinSettings.php
index 0bb6e42..15a111f 100644
--- a/src/GinSettings.php
+++ b/src/GinSettings.php
@@ -27,7 +27,7 @@ class GinSettings implements ContainerInjectionInterface {
   /**
    * The user data service.
    *
-   * @var \Drupal\user\UserDataInterface
+   * @var \Drupal\user\UserDataInterface|null
    */
   protected $userData;
 
@@ -41,15 +41,15 @@ class GinSettings implements ContainerInjectionInterface {
   /**
    * Settings constructor.
    *
-   * @param \Drupal\user\UserDataInterface $userData
-   *   The user data service.
    * @param \Drupal\Core\Session\AccountInterface $currentUser
    *   The current user.
    * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
    *   The config factory.
    */
-  public function __construct(UserDataInterface $userData, AccountInterface $currentUser, ConfigFactoryInterface $configFactory) {
-    $this->userData = $userData;
+  public function __construct(AccountInterface $currentUser, ConfigFactoryInterface $configFactory) {
+    if (\Drupal::hasService('user.data')) {
+      $this->userData = \Drupal::service('user.data');
+    }
     $this->currentUser = $currentUser;
     $this->configFactory = $configFactory;
   }
@@ -59,7 +59,6 @@ class GinSettings implements ContainerInjectionInterface {
    */
   public static function create(ContainerInterface $container) {
     return new static(
-      $container->get('user.data'),
       $container->get('current_user'),
       $container->get('config.factory')
     );
@@ -121,7 +120,7 @@ class GinSettings implements ContainerInjectionInterface {
    *   The account object. Current user if NULL.
    */
   public function setAll(array $settings, AccountInterface $account = NULL) {
-    if (!$account) {
+    if (!$account || !$this->userData) {
       $account = $this->currentUser;
     }
     // All settings are deleted to remove legacy settings.
@@ -137,7 +136,7 @@ class GinSettings implements ContainerInjectionInterface {
    *   The account object. Current user if NULL.
    */
   public function clear(AccountInterface $account = NULL) {
-    if (!$account) {
+    if (!$account || !$this->userData) {
       $account = $this->currentUser;
     }
     $this->userData->delete('gin', $account->id());
@@ -164,7 +163,7 @@ class GinSettings implements ContainerInjectionInterface {
    *   TRUE or FALSE.
    */
   public function userOverrideEnabled(AccountInterface $account = NULL) {
-    if (!$account) {
+    if (!$account || !$this->userData) {
       $account = $this->currentUser;
     }
     return $this->allowUserOverrides() && (bool) $this->userData->get('gin', $account->id(), 'enable_user_settings');

🇫🇷France NicolasGraph Strasbourg

I suggested a comment update ; otherwise, it seems good enough to me to go forward.
Thank you both!

🇫🇷France NicolasGraph Strasbourg

Thanks for your work @MartinPJB, could you just remove the .patch file from your changes please before we go forward please?

🇫🇷France NicolasGraph Strasbourg

This seems a quite good approach to me and it does work on a fresh install of mine.
I'm just wondering: why to use a twig filter to build and clean the class instead of just doing it properly in the GinNavigation service ?

🇫🇷France NicolasGraph Strasbourg

Here is a patch.
The "Create" menu item can now be a simple span if the node module is disabled but another content entity type exist as a submenu.
Complementary styling could be useful for this "span state" I guess, but it does not throw any fatal error anymore.

🇫🇷France NicolasGraph Strasbourg

Here is the patch I talked previously, however, in my case, .getUntranslatedString() returns NULL for some links title, so in fact some icons are still missing. Falling back to the title works for a few of them, but still not ideal.

🇫🇷France NicolasGraph Strasbourg

Not sure that's the best way to do but I was able to locally fix the issue by replacing toolbar-link--{{item.title|clean_class }} by toolbar-link--{{item.title.getUntranslatedString()|clean_class }} in templates.

🇫🇷France NicolasGraph Strasbourg

I had the same issue concerning the disabled help module and the patch from #9 did the trick.
I also confirm the issue pointed in #10 about translated classes and missing icons; this probably needs a dedicated issue.

Production build 0.69.0 2024