Account created on 15 July 2008, over 15 years ago
#

Merge Requests

More

Recent comments

🇮🇪Ireland markconroy

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

^ That's a good point.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

@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.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

This looks great. Thanks @finnsky.

RTBC from my point of view.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

@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.

🇮🇪Ireland markconroy

LGTM, let's get this merged. Thanks.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland 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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.
🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

Marking this as RTBC.

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

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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".

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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?

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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:

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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?

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

Very nice work. Thanks very much.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

@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.

🇮🇪Ireland markconroy

@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?

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

This is fantastic news

🇮🇪Ireland markconroy

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.

🇮🇪Ireland markconroy

Updating issue summary to remove

  1. Steps to reproduce - these steps do not reproduce the issue, since the issue is RTBC and the SVGs are not presenting a problem and not changed at all in that MR.
  2. Proposed resolutions - we have not agreed to proceed with the resolution in [#3]
🇮🇪Ireland markconroy

Interesting approach @mherchel.

I'm not fully sure if we need to "fix" this issue since Umami has a set design and the colours are set in the SVGs. It only became an issue because one of the people working on the MR wanted to declare the CSS variables inside the SVG - which would mean the same variables declared in CSS and then multiple times in SVGs, which doesn't look very DRY to me. Instead, I decided we'd leave the Umami SVGs as they were and create this follow-up IF we needed it.

My preference would probably be to inline the SVGs instead of working with CSS workarounds, and then we could use whatever CSS we wanted to to add the colours for them.

Moving this to "Needs work" instead of "Needs review" since there is no actual code to review.

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