🇬🇧United Kingdom @scott_euser

Thanks for the feedback! Comments address + update test added + 2nd change record added

1. Merged 11.x into the branch
2. Retested with Footnotes module (disabling the fix that the module itself is applying)
3. Balloon within modal appears again:

No concerns here, looks fine to me! I haven't actually tried re-indexing. Minor comment added only

This is great, nice to be able to handle giant single nodes, e.g. guides or books perhaps. Added some comments to the MR, but the only general thing is I think we probably need to make this an opt-in given there are some less configurable hosts well used in the drupal community like Pantheon that have very infrequent cron

Looks good to me! I created ✨ In 'Add fields' UI, auto-expand level 3 if level 2 has only one item Active for now, maybe you can have a look there and see if that is what you mean

scott_euser → created an issue.

scott_euser → created an issue.

From Google's docs it doesn't look like that's an option, that link describes a single domain or wildcard only.

But maybe there is a way to add a validation step prior to Drupal allowing the authentication to create the user account, ie just at the stage where Google has returned the authorised email to bail out there. Haven't explored that myself though.

I don't think you can restrict multiple domains; at least according to the docs not BUT it does look like you can restrict to accounts owned by a Google Cloud Organization according to the docs: https://developers.google.com/identity/openid-connect/openid-connect#hd-...

I have not tested it but setting it to * should cover that.

scott_euser → created an issue.

scott_euser → created an issue.

scott_euser → created an issue.

Sorted at least for now, can discuss multistep framework later if you are interested. Will move on to test coverage & coding standards now.

Retested in Drupal 11 from start to finish. All working good.

Re-tested process from start to finish after merging from 1.0.x latest.

I am on Drupal Slack at scott_euser if you want to discuss; I don't want to overstep and so would be good to understand how involved you want to stay. For now I'll just merge bugfixes.

Cheers thank you!

I wonder if its worth just altogether switching to https://www.drupal.org/project/multistep_form_framework → as perhaps the whole thing is better of being ajaxified not just particular steps.

scott_euser → created an issue. See original summary → .

1. Submit handler added
2. Revalidate to ensure user doesn't manipulate form

scott_euser → created an issue.

There you go, shows error if validation fails. Otherwise proceeds to step 3.

scott_euser → made their first commit to this issue’s fork.

Thanks for the patching on this, was a good starting point. Updated to match Drupal Core user module to support the same tokens, otherwise the email is much more likely to just go into spam. Would say this is 'Critical' as the module does not function at all without this.

Hiding patches as per https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →

If this looks okay to others please RTBC

scott_euser → made their first commit to this issue’s fork.

Tested in clean drupal 11 install. This works fine, though suffers from 🐛 OTP Email not send Active as well. Anyways that can be addressed there, so this is ready to merge (ie, branch 'project-update-bot-only')

scott_euser → created an issue.

Nah all good, does no harm, ignore me :) Anyways I put in an application to take over maintenance since the maintainer has been quiet for a long time now. If that does go ahead I'll get a bunch of issues merged in + test coverage running again once 📌 Automated Drupal 11 compatibility fixes for simple_cron Needs review is in

Thanks! CronTest.php phpcs seems like its out of scope, I don't think phpcs should be applied to existing code as part of this issue, that is getting resolved in other issues.

I couldn't actually see the GinToolbarNegotiator - should it not be there too?

Thanks for the work on this!

1. The latest branch with commits has merge conflicts, ie, 3217393-add-permission-for
2. There seem to be multiple branches: the other branches that are unused should be hidden
3. I believe we should also have an update hook so the status quo is maintained for users. Ie, if we previously expected users with 'access toolbar' to have access and all of a sudden they don't, we cause a problem updating. So we should loop through roles and grant primary & secondary permission to any role that currently has 'access toolbar'

Updating as per #8 + hiding patches as per recommended advice to download diff to your local repo.

When I tried updating to this I get the error:

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'db.ai_log__field_messages' doesn't exist: INSERT INTO "ai_log__field_messages" ("entity_id", "revision_id", "bundle", "delta", "langcode", "field_messages_value") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => 2 [:db_insert_placeholder_2] => chat [:db_insert_placeholder_3] => 0 [:db_insert_placeholder_4] => und [:db_insert_placeholder_5] => user Test message. Say hello bacck )

Of the following exception type:

Drupal\Core\Entity\EntityStorageException

despite ai_logging_update_10305 and ai_logging_update_10306 successfully running

I believe this is because the field storage actually needs to be installed rather than just importing the config. I have some examples of doing that in site_settings module such as site_settings_update_10003 if it helps!

After that I can retest

Okay sounds good thank you! Created a new issue here 💬 Offering to co-maintain Simple Cron Needs review

Updated after contacting Edvinas. I attempted to find him via Drupal Slack as well but no joy there.

scott_euser → created an issue.

Phpstan issue is already covered here: 🌱 The method \Drupal::moduleHandler()->getName($module) has been deprecated Active

Perhaps the configuration form could have a selection based on role or select a particular user. Based on role the problem is the code would create then clean up a user, so user IDs would increment. Based on user the problem is user could get deleted (though I suppose it could fallback to status quo if that happens).

There are more places (updated MR) plus useful to keep the label still. However this needs work as tests also need updates. It makes sense to first get 📌 Automated Drupal 11 compatibility fixes for simple_cron Needs review in before updating as tests fail in general at the moment.

scott_euser → made their first commit to this issue’s fork.

Merge conflicts, needs update

This will block upgrades to Drupal 11 soon now that we are in D11 beta. Not sure correct status, but hopefully Needs Review is the right one

Fine to create as a separate issue I think. This one is okay, but coding standards should be followed (which currently are not). Updated issue summary to reflect that.

Certainly doesn't hurt to be more protective in the code like this.

This looks good to me, but a couple suggestions as we don't need to be backwards compatible to drupal 8 any more. 10x only needed.

📌 Automated Drupal 11 compatibility fixes for simple_cron Needs review

Once we have 🐛 Views handler loading should respect configuration Needs review and ✨ Configurable views filters to allow for different widgets Active in so Views filtering allows selection of which widget to use stopping us having the problems described in #215 and #225 by allowing this to be a new filter (and potentially in a follow-up deprecating the old filter)?

To note, pages that list other things can still be useful, like /admin/structure/views is quite essential as many sites will have too many views to list in the menu + there is plenty of other useful information in that table that can help site builders choose which view edit that they lose out on if they stop being able to access it (fully aware that hiding it does not 'stop' them accessing it but certainly makes it hard to get to). I believe that would also be solved if we follow @laurii's suggestion in #30.

Wonder if we can make it a configuration option in Views table display plugin? So opt-in?

https://www.drupal.org/project/markdown_field_formatter/issues/3479806 📌 Support for Drupal 11 Active

Thank you all for all the hard work on this. Challenging problem and sounds like we need some sort of round table or vote or something perhaps from a slightly wider group. Is #admin-ui slack channel maybe appropriate to have it discussed as part of the next meeting to agree on a direction?

I have been working on ✨ Improve the Search API admin UI for adding/editing fields Active which is nearly there (just coordinating final steps via slack with solr and search api maintainers). As a next step I would love to be able to leverage this as I am sure you have probably all tried search API add fields and know how unweildy that list can become. With search api getting into Drupal Cms (and AI Search leveraging that as a submodule of AI) it'll be a great improvement for both.

So I would love to help test this out within the context of core and test extensibility of it over there (if it's not @internal), but sounds like a bit of a huddle is needed first to know which direction to test.

In any case this is a really great chunk of work that could benefit a lot of areas I'm sure! Thank you!

PHPCS issue is unrelated existing in 3.0.x-dev as well.

Demo of this:

1. Default: only one style
2. Create alert, no style shown, but style still correctly chosen on list of alerts
3. Add a second style
4. Create alert, choice given

scott_euser → created an issue.

Note that you need to update to the latest MR in ✨ Improve the Search API admin UI for adding/editing fields Active if you are rechecking.

Fixed as per discussion in Slack; thank you for helping me reproduce the error and understand the purpose. I added a comment to the MR to explain the purpose here for future travellers.

Okay thanks for talking me through it! Happy to help on implementation when we get to that stage (that does look simpler to achieve)

What would be useful though is RTBC'ing it if you have reviewed (+explaining what you did to test/review). Thanks!

Hiding patch, you should download the patch locally as per https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → This avoids confusion for maintainers & further development as to what to work on.

Thanks!

That sounds sensible. I would be happy to work on that but sounds like we should get ckrina's opinion first? If its a go for that route, would that be okay to add to this issue's scope?

If in Core it sounds like maybe we need a mechanism either to opt in or opt out of having a direct access link. If opt in would need to update a lot of a contrib projects but at least that makes it a conscious decision. Perhaps a new attribute in menu link?

I suppose it could be in contrib extending the template and overriding the css?

In any case thank you very much for the reviewing and tips!

1. Sorry I didn't quite follow, where should I add a required label? Or is that a process FYI thing?

2. Fixed:

3. This seems to work: if you tab to the down arrow and press right/left it opens closes. I am not sure it makes sense to have both the button and link both respond to the right/left arrow? If so, do you have any tips? I tried duplicating these bits from the down arrow:

'aria-expanded': 'false',
'data-toolbar-menu-trigger': menu_level,

I might be a bit in over my head there if that is what you are expecting.

4. Changed to SDC, thanks!

This doesn't have all comments from #5 resolved so moving this back to need work. E.g. No test coverage

Yeah duplicate, guess we should close this one to avoid confusion

scott_euser → changed the visibility of the branch 3.x to hidden.

Seems like random unrelated test failure (seeing that sometimes lately), rerunning

You can also view it at https://mr10134-y6ngslwhd9aeawkbkod3pwnb7vnmyrr3.tugboatqa.com/user/login + https://mr10134-y6ngslwhd9aeawkbkod3pwnb7vnmyrr3.tugboatqa.com/admin after logging in

Ah sorry, pushed to the old closed branch! Fixed now

Just wanted to check if we are considering how to navigate to the page if it has children. Ie, to resolve 🐛 Second level menu items can't be reached if they have children Active type of problem where if something has children you can only navigate to the lowest child then navigate back up via breadcrumbs.

Maybe this is tying loosely into 💬 Support Deeper Navigation Levels Active

Looks good, thank you!

scott_euser → made their first commit to this issue’s fork.

1. Updated code to work with new code from 🐛 drush migratation with --sync has suboptimal performance (v6) Active
2. Added an example source to the test coverage
3. Added additional test coverage method to prove the issue and demonstrate the fix: test only fails.
4. Updated issue summary to standard template with details and clear steps to reproduce

scott_euser → changed the visibility of the branch 3104268-sync-id-too-strict to hidden.

scott_euser → changed the visibility of the branch 3104268-sync-might-be3 to hidden.

Okay rest are now ones that --fix cannot fix, then needs a bit of manual testing.
Had to ignore ai_chatbot for now, just makes eslint hang

scott_euser → created an issue.

Side note, this makes me think we should remove the SKIP_ESLINT and sort that. Its hard to review the code for this as its so much coding standard change

Wow this is an old one; must have been one of my first attempting to do tests :) Rereading wondering if its better just to close this given #59. Deleting a transition and creating it again really only typically means having to redo the permissions, but given its likely to happen while you are setting up a workflow, its likely you have not yet gotten to permissions yet in the first place. Feel free to re-open if disagreeing.

Thanks for checking; yes I can see the bug - I fixed it now + added more test coverage to validate.

Looks like you need to clear your cache - both the twig template and css change. Thanks!

Further updated if you use `[node:url-LANGCODE]` you will get a fatal error otherwise because ->url() exists on translation but ->toUrl() is needed for entity interface

Yep looks fixed

scott_euser → changed the visibility of the branch 2956982-port-hreflang-tokens to hidden.

scott_euser → changed the visibility of the branch 2.1.x to hidden.

scott_euser → changed the visibility of the branch 8.x-1.x to hidden.