Account created on 15 July 2008, about 16 years ago
#

Merge Requests

More

Recent comments

🇮🇪Ireland markconroy

Hi @joelseguin

I have a merge request ready for this. Can you test it please?

https://git.drupalcode.org/project/content_access_by_path/-/merge_reques...

The only file you need to look at is the changes in the .module file. All the other changes are just coding standards fixes.

Thanks very much.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Thanks for posting this @joelseguin. I'll work on a fix for it.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

I'm going to close this issue as "out of date" since we have separate issues created for each of the requests in this one.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Thanks @smustgrave, this took me a little bit of time to track down.

I've added a new commit to change the string that we should search for. This is because since we set the nodes to have different dates on them in the new install hook, the string we were searching for belongs to a node that will not make it to the front page listing.

The ordering for that view on the homepage is

Sticky at top of lists => Date added => node id

With the date added now changed, and the node id for this content item being 1, it no longer makes the grade :)

All tests are passing now.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

I'm going to mark this as "Needs more info".

We can make the change quite easily if we do it for D11 only, but we'd need to do that fairly quickly before we release D11 in the coming weeks.

Failing that, I think we need more info about why they were moved to the media directory.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Hi @smustgrave,

Looks like we need to keep the line to override the off_canvas library in the $librariesToOverride array.

Tests are passing now.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

I've tested this on Firefox and on Chrome on desktop and using the "responsive view" as per the IS instructions and can't find the issue still happening.

Also, all of the screenshots are from the Seven theme, so I'm inclined to think this is a Seven theme issue, rather than a stable9 theme issue.

I'm going to close this as outdated. Feel free to re-open if you can provide steps to reliably reproduce the issue.

🇮🇪Ireland markconroy

I think I've come back out of the rabbit hole and tracked down the correct issue here. It's not that stable9 is trying to reference the wrong files, stable9 does not have any CSS files any more for off_canvas, they were all removed by c2af57548.

So when we try to run the tests, specifically Stable9LibraryOverrideTest, it's looking for CSS files in stable9/css/core/dialog... which are no longer there.

Let's see if removing that section of the stable9.info.yml file and the corresponding library from $librariesToSkip fixes this issue.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Setting to Needs review again, I just want to check that we don't have a red herring test failure.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Also, the patches we have for this so far are for stable and stable9, but stable is not part of Drupal core any more. So at a very minimum we need to update those to refer to stable9 only.

🇮🇪Ireland markconroy

I don't think this is an issue any more.

After installing standard profile, I tried the following tests:

  1. Set stable9 as my admin theme, uninstall contact module (via UI). No errors reported.
  2. Set stable9 as my admin theme and default theme, uninstall contact module (via UI). No errors reported.
  3. Set stable9 as my admin theme and default theme, uninstall Claro and Olivero so only stable9 was left enabled, uninstall contact module (via UI). No errors reported.

If this is still an issue, we will need an updated issue summary and steps to reproduce.

If this is still an issue, I suggest instead of removing this line
The following reason prevents {{ module.module_name }} from being uninstalled:

We look to set {{ module.module_name }} to be a simple variable, so just inside the {% for %}, something like this:

{% for module in modules %}
  {% set module_name = module.module_name %}
{% endfor %}

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Thanks @finnsky

I've made some amends to your fixes.

Moved the .footer class back to the <footer> element
Set .layout-footer class to use the CSS that was targetting .footer

I think this is looking pretty good now.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Hi @mgifford

Can you have a glance at this please?

The MR moves tabs/highlighted/content-top/etc all inside the <main> element. And then amends the CSS to target a new container I created inside this called - .main-content-area so that sidebars, etc continue to work.

However, I am not sure if it's best for us to move all those items inside <main> or if, perhaps, we should create a new <section> above <main> and place them in there instead.

Thanks very much!

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

I'm working on this.

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

Thanks @needs-review-queue-bot

Fixed, and setting back to needs review.

🇮🇪Ireland markconroy

I wonder could we do something like a post install update that will cycle through all the nodes on the site and set them as created a day between each.

So if we have 30 nodes, then

  • node/30 will be set to have been created yesterday,
  • node/29 will be set to have been created 2 days ago,
  • node 28 will be set to have been created 3 days ago,
  • etc

---
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Closing, as no longer relevant.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Closing, as no longer relevant.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Closing, as no longer relevant.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Marking as "Needs review" to let the bots do their dance.

🇮🇪Ireland markconroy

Updating to add credit to Code Enigma.

🇮🇪Ireland markconroy

Thanks so much, @b_sharpe.

I'll create a new branch to remove what is currently there and replace it with my version.

🇮🇪Ireland markconroy

At DrupalCon Prague we decided that field--has-multiple-items and field--has-single-item were not communicating the correct information. We need to communicate the field cardinality in the database. When a user sees has-multiple or has-single they will connect this information what is rendered. A multivalue field can have just one entry, but in the original patch it would get a has-multiple-items class.

Just for clarity, the way field.html.twig currently works is that if it's a multivalue field - even if it only has one item in it - it gets the .field__items wrapper. If it has only one item, that item is wrapped in a div and if it has more than one item it uses ul.

The class we are intending to add here field--multiple will denote that this is a multivalue field, not how many items are in the field.
Perhaps we could add another class field--has-N-items to denote how many items we actually have.

Suggestion for classes:

  'field--name-' ~ field_name|clean_class,
  'field--type-' ~ field_type|clean_class,
  'field--label-' ~ label_display,
  multiple ? 'field--multivalue-field' : 'field--single-value-field',
  multiple ? 'field--has' ~ items|length ~ 'items'|clean_class : 'field--has-1-item',
  label_display == 'inline' ? 'clearfix',

Note: we will need to update the issue summary with new notes for the change record.

🇮🇪Ireland markconroy

I think this can be marked as "Closed works as designed", given it's for D7 and hasn't had anyone look at it for over 7 years.

🇮🇪Ireland markconroy

I'm going to close this issue since it belongs to Drupal 9.5 which is no longer supported and our `/core/.stylelint.json` file for D10/D11 does not include the `all` property. in the "order/properties-order" definition.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Hi @YesCT

Do you have steps to reproduce this issue for us?

Also, your commit to add Tugboat looks really valuable. I wonder could you create a separate issue/MR for that, so we can get it merged while this issue is still in progress?

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Tested againsta 11.x.

LGTM.

===
Thanks to Code Enigma for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Thanks for posting this issue @Lillian Bozeman.

I'll get a fix for it asap.

🇮🇪Ireland markconroy

Adding patch file to use in current projects.

🇮🇪Ireland markconroy

Adding credit to Big Blue Door who sponsored the time to work on this.

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

🇮🇪Ireland markconroy

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?

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

Production build 0.69.0 2024