🇮🇪Ireland @markconroy

ekes → credited markconroy → .

This merge request seems to rely heavily on using .toArray(), but I don't think that is supported in Firefox or Safari.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global...
https://caniuse.com/?search=toArray

I just want to check that our build tools have something in place to transpile this to FF/Safari-compatible code?

Besides that, we'll done, this is a very large piece of work.

I think this looks like a great idea, but now I also have a worry about us having a field called body and a field called field_body if someone tries to add a new field to a content type and uses the "reuse existing field" approach. Also if you create a new content type it will automatically get the body field added.

I guess people don't need to be creating new fields and content types, since this is a fully formed demo.

I wonder could we disable the default body field from the UI fully?

nod_ → credited markconroy → .

and prevent clicking on a second facet prior to loading the new page.

^ That's a good point.

I'm not sure exactly why we need to disable the checkboxes when we are creating checkboxes from links for facets, so I created this small patch to remove the disabling of the checkboxes and it seems to fix the back button issue.

RTBC!

Let's get this merged!

Hi @xiwar

I tried your branch, but the module wouldn't install from it. Also, I am a little worried without more testing about using Link field. I get that it will work fine for the individual node, but I have a concern about nodes that use the current node as part of their paths without giving it a lot of testing. For example, if you are allowed to edit "Rugby" and the URL is /sport/rugby, will you also be able to edit /sport/rugby/world-cup (as you should be).

In the mean time, I fixed the specific issue we see (thanks for the detailed report), by:

1. Checking if $matches[] has a value,
2. If so, doing the normal thing, and
3. If not, setting NULL as the query value which forces the empty view message to appear. And then I've amended that view message to "Sorry, there is no content assigned for you to edit."

I'm going to mark this as fixed and create a new release. It might be good if you think it's worth it to create a follow-up issue titled "Change paths text field to a link field" and we can work on it from there.

markconroy → changed the visibility of the branch 3418083- to hidden.

Hi @xiwar

Thanks for reporting this and working on it. I've tried in a number of ways to replicate it but can't. Example:

1. Create Subsite overview node called "Test Subsite", with URL /test-subsite
2. Create term called "Test Subsite" in "Content Access by Path" vocabulary, with path test-subsite
3. Apply this term to a user
4. Go to /admin/content and see that the only nodes I can edit are my own nodes and the "Test Subsite" node
5. Edit "Test Subsite" node to call it "My Test Subsite", so the url is /my-test-subsite
6. Go to /admin/content and see that node is no longer available for editing

@Frontmobe, yes I uninstalled the module and just left the taxonomy intact.

However, be careful. If you uninstall the module it will also delete the vocabulary, so you need to go the the countries_info.install file and comment out the part that will delete the vocabulary. Then you can uninstall the module and remove it from your codebase.

Done!

https://www.drupal.org/project/content_access_by_path/releases/1.0.0 →

Hi Jim

Yes, I'll be opting in to security coverage but can't apply for that until the module is at least 10 days old.

I'll be able to apply from Monday 29th

markconroy → created an issue.

Fixed in MR2

markconroy → created an issue.

Fixed by 8f884a8

markconroy → created an issue.

Fixed by 7aa2d5ab9

markconroy → created an issue.

Merged. Thanks.

Yes, this only started when I updated to 10.2 last week. Or at least, we've only noticed it since then.

In my case, I have an image field on a node (not in a paragraph, just a general node field). When the media library modal opens, the "Insert" button looks fine, but if I use any of the filters and click "apply" (or even click apply to the filters without having changed any of them), then the .ui-widget library seems to get loaded after the claro library, so I get the same issue that is reported here.

I am getting the same issue when using a media field as just a field on the node, rather than as a media field inside a paragraph. So I don't think this is a paragraphs issue, I think it's a core one.

Note: there is a paragraphs field on the content type, but even creating a new node and not filling out any of the paragraphs fields I am still getting this issue.

markconroy → created an issue.

I'll mark this a works as designed. Thanks folks.

This looks great. Thanks @finnsky.

RTBC from my point of view.

I think this is a great idea. And I love the potential for our custom components to be used in other setups, such as if we had a custom menu component, I can imagine other developers using it in their personal projects or other CMSs learning from it, using it, or adapting it to make it suit their. use case.

@saganakat you can't patch a .info.yml file.

The file might look like this when the patch is created:

name: My Module
description: A module that does something
core_version_requirement: ^9 || ^10

But after Drupal packages it, it looks like this:

name: My Module
description: A module that does something
core_version_requirement: ^9 || ^10

# Information added by Drupal.org packaging script on 2023-09-29
version: '3.4.2'
project: 'my_module'
datestamp: 1696006156

So git sees it as having changed and can't patch it any more.

LGTM, let's get this merged. Thanks.

This is one of those recursive issues

1. Too much space when editing in Claro: #3158854: Node form: address chasm between main form and meta →
2. The node edit form is now too wide in Claro: 📌 Node form layout looks awkward on wide screens since #3158854 Fixed
3. Today: The node edit area is too thin in Claro: 🐛 Claro content editing area is too narrow Needs review

We've found it, solved it, found it, solved it ...

I'm not sure what the actual answer is. I know in Annertech we just created a clone of Claro to add the CSS to fix this specific issue for our desires because it seems whatever we choose it will be too wide for some and too narrow for others.

Yep, looks like I missed doing that. I'm happy for my application to be withdrawn since I am not using this module currently on project.

Note for future applications, rather than a blanket "I am closing this offer, since there are not been any follow-up action as described in ..." message, we could do something like:

"Hi Mark,

There's 6 steps to becoming a project maintainer. You haven't completed step 6 yet. If you are still interested in being a maintainer, can you get that completed. If not, will we close this issue."

I think that might be friendlier for those offering to maintain projects.

Hi @apaderno

Was there a follow up item I was supposed to work on? If the action was waiting for the current maintainer to respond, well they haven't been active on this module for 4 years.

I don't mind not being maintainer of this module, as I created a work around not to need it for Drupal 10, but I wasn't aware there was any thing else needed of me to apply to be co-maintainer.

markconroy → created an issue.

stella → credited markconroy → .

John Cook → credited markconroy → .

Since this module is not receiving any updates, I don't see the point in having it in my codebase any more. However, if you uninstall it, it deletes your "Countries" taxonomy and also removes all the terms/countries in it.

To get around this, you can:

1. Open countries_info.install file
2. Delete the countries_info_uninstall() function
3. Uninstall the module
4. Deploy this change to your website
5. Remove the module from your codebase
6. Deploy this change to your website

Sorry, I know this isn't helping get this issue merged, but it's a workaround for you to get your site to Drupal 10.

Just a note for anyone reading this/working on this. The conversation seems to have moved towards talking about Olivero, but this issue/patch is for Olivero and Umami theme and starterkit_theme.

That said, "works as designed" seems the right status for this.

Ooops, sorry. I had forgotten this wasn't my issue and I was hijacking it. I've created 💬 Offering to co-maintain Countries info by markconroy Active now to see about becoming a maintainer myself for this module.

markconroy → created an issue.

I understand how the process works, I'm a maintainer/co-maintainer of a number of modules and also a Drupal core maintainer for the Umami profile and theme (Out of the Box initiative). However, in the case of this module it's a little hard - there are very few open issues (50% of which are now RTBC), one which may not be an actual issue, and one which is simply a feature request, so it's hard to demonstrate being "actively" involved in a project's issues, if that project is to all intents and purposes abandoned.

• I've made a comment on issues in this project and moved one to RTBC (there are very few issues) and all we really want is a D10 release.
• I've emailed the maintainer of the project and asked them if they can make a D10 release.
• I've offered here to become a maintainer of the module.
• I've offered to become a maintainer of the module to the project owner as well.

You can put me down as co-maintainer if you wish. I have permissions to opt-into security advisories.

To be honest, all I want to do currently is mark the .info.yml file as ^9 || ^10 so we can upgrade sites using this module.

I think it's important that this module gets a new maintainer or a co-maintainer. Looking at the commit log, the last commit to the module was 3 years ago and all that commit did was fix Drupal 9 deprecated errors so it could be compatible with D8 + D9.

It doesn't look like the current maintainer is actively maintaining this module.

Marking this as RTBC.

Any chance we could get a new release of this module soon please?

Actually, let's close this issue since it is a duplicate of 📌 Automated Drupal 10 compatibility fixes Needs work

Setting to needs review so the bots can do their thing.

Great, thanks @nilesh.k

Let's move this back to RTBC pending a frontend framework manager review to check the comments in #68 and #69

Marking as "Cannot reproduce"; the spacing seems fine to me and comment #5

Setting back to 'Needs review' to see if #64 passes, but also so we might get some feedback from a frontend framework manager given our concerns from #68 and #69

Hi @andrewmacpherson and @mgifford

Just want to check that this is an issue we need to fix. The "motion" we have for hover and focus states is a very subtle motion to change colour and/or show an underline/border. This is a far cry from things zooming in and out of the page. If someone "prefers _reduced_ motion" I can understand on a site with things zooming in and out that they would want this "reduced".

I'm not sure if our motion is so severe that we can reduce it at all without actually removing it. Is that what we want to do in this issue - set "no motion/transition time" for items if "prefers reduced motion" preference is set? I'm perfectly happy if this is the desired state we want, just want to be sure before we work on it.

Thanks @catch

This should definitely NOT be removing "Experimental" from the profile name. Let's get that put back in before we move this to RTBC. With that being the case, I'm removing the "Needs product manager review tag".

I'm going to close this as "works as designed".

Umami maintainer here!

I'm fine with this being closed as "Works as designed" as we are perpetually experimental and not providing updates.

However, to fix the issue reported, I reckon you can simple install the SDC (Single Directory Components) module that is available (also experiemental for now) from Drupal 10.1. Can you check if that fixes your issue please?

MR13 solved the issue for me.

Wow @stBorchert, I wasn't expecting a reply so quick. Thanks very much. That fixes the issue.

markconroy → created an issue.

Thanks for looking into this @stBorchert.

It's very strange. I have uninstalled paragraphs_ee and paragraphs_features, and then re-installed them again but this doesn't fix the issue. The only paragraphs modules I have installed are these two and also paragraphs itself.

Then I realised many of my content types have more than one paragraphs field on them, so maybe that was the issue, so I created a new content type with just one paragraphs field, but I get the same result.

We have only one custom module on the site, which I have disabled, so that is also not the issue.

Oooooh, but then I realised we have a custom sub-theme of Claro. When I set Claro as the admin theme the issue seems to be fixed. So the problem lies in our sub theme it looks like. Sorry for all this confusion.

We're using Claro.

Cool. Let's close this so.

Hi @stBorchert

I have upgraded the site now to Drupal 10, I've upgraded paragraphs_ee to 10.0.0 and I have "add in-between" enabled, but still not getting any joy.

Form Display:

Node edit form:

I have it set to "Add in between" and that just means that in the first "add in between" instance, you see all the buttons like in the screenshot above, but then there are no other "Add in between" items.

Would be great to get a fix for this. It's the last module blocking an update to D10 that I have on-going.

I have it set to "Add in between" and that just means that in the first "add in between" instance, you see all the buttons like in the screenshot above, but then there are no other "Add in between" items.

I'd prefer if the core version requirement was ^9 || ^10. We can't be sure this will work for D11, D12, etc.

In any case, >=9 is RTBC and committed and released, so good enough for me.

The patch in #11 says "this module is only compatible with Drupal 9 an Drupal 10", it doesn't give any indication that it's compatible with Drupal 8.

-core: 8.x
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^9 || ^10

Let's look towards getting that merged and release. We shouldn't have such a long running issue for a module that is only 1 line of code.

the patch in #15 looks good to me, as it also adds a composer.json file, but I'd gladly accept the patch in #11 either.

Can we get this merged and released soon please?

In our case, we don't have a boolean field in this content type, but we do have a list field. I'm going to guess that's what is tripping us up here.

Very nice work. Thanks very much.

As an Umami maintainer, I don't think this feature request will be accepted.

Umami shows lots of features of Drupal core, layout builder is one of them. However, we also want to show that there are other ways to manage layout, such as templates and also the old fashioned "Manage display" page.

Let's leave the top and left CSS as is. If we use inset, then it's in effect saying "top: 0; right: 0; bottom: 0; left: 0" and in that case we should also remove the width and height, since inset will have taken care of it.

Let's not hold this issue up any more, looks like enough maintainers and committers are happy with it as is. We can create a follow up to rewrite our CSS in more modern, logical property format.

It's a very standard set up. We have a field for posting to twitter, and we use this module to let editors know to keep their post to fewer than 280 characters. Editors fill in that field and then the contents of it are posted to our Twitter account.

When I try to edit any page of this "social post" content type, I get the error mentioned in the OP. If I add return $value_length to the getLengthOfSsubmittedValue, it's working fine. If I downgrade the module to a previous version, it's also working fine.

I can try upgrade it again, maybe I did something wrong! Will report back.

I think the issue is caused because there is no return $value_length; in protected static function getLengthOfSubmittedValue

markconroy → created an issue.

Thanks for working on this. I've no issue with our Tour items being removed if Tour is being removed from core. But let's definitely not support Tour as a contrib module in our demo_umami profile - so let's remove _all_ of Tour from Umami and not have some of it supported via "optional" config.

markconroy → created an issue.

New patch incorporating #16 and my updates from #20 (with some amendments).

@heddn for the current issue, let's not expand the scope of it too much. It might be a good idea, however, to open a new issue to have 2x and 3x multipliers added for Umami images.

@mherchel Darn, looks like you are right, not sure how I missed #16

In any case, I think I prefer the HTML and the CSS of my #20. How about we take your config and my template and CSS?

Marking as duplicate of 📌 Automated Drupal 10 compatibility fixes Closed: outdated

New patch after running the CSS linter. Interdiff is against #15.

Here's my suggestion

1. Rewrote the template to use BEM classes
2. Added container class for the summary area
3. Rewrote the CSS to use GRID so things stay withing the DOM flow

As I said in the previous message, I'd prefer us to _not_ use 7:3 aspect ratio for all the image styles in this component and would rather if we had different dimensions/ratios for different view ports.

I really like the general direction of this, but I think we've messed up the CSS a bit (testing on 10.1, as I think it's okay for Umami to bring in new changes in point releases).

Small screen - looks fine

Medium Screen - image becomes a big square taking over the full screen, not the "Read article" link in bottom left, as it's covering the cards that come after it

Large Screen - see how the image is covering the cards that follow after this component.

I'll try fix this up now, but then I think I'll propose that we remove the new 7:3 responsive image style and create one called "Hero", so we can have a portrait image for small screens, some other ratio for tablets, and 7:3 for large screens which I think will be more in tune with the designs.

I've no problem with us changing the text, but as an Umami maintainer, I'd vote for "Directions" instead of "How to make it".

I also don't have a problem with us just changing the field label. I often have field machine name such as field_teaser and field label such as Teaser text (max 150 characters). This might also be a good example of how Drupal can handle different field names vs field labels.

I don't think this is a bug, I think this is the actual design. If you look at it (and use a screen ruler) you can see that the "Menu" text aligns with the bottom of the "Site name" text.

That said, I do prefer the look of the menu position when the patches in this issue are applied, with the menu icon/text centered vertically.

Looking forward to Mike Herchel's 2c.

Thanks @mortona2k

This looks really good now. I've only read through the code, not actually tested it yet. I'll try get it tested later this week.

This is fantastic news

Sorry @moshe weitzman

I've just been snowed under with work for the past couple of weeks and haven't had a chance to get back to this yet.