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

Merge Requests

More

Recent comments

🇮🇪Ireland markconroy

@willguv you could set up one for Friendly Digital if you wanted.

🇮🇪Ireland markconroy

Closing this individual issue, in favour of just one credit on the aggregated issue: 📌 Localgov Base - fixes eslint issues across the project Active

🇮🇪Ireland markconroy

Closing this individual issue, in favour of just one credit on the aggregated issue: 📌 Localgov Base - fixes eslint issues across the project Active

🇮🇪Ireland markconroy

Closing this individual issue, in favour of just one credit on the aggregated issue: 📌 Localgov Base - fixes eslint issues across the project Active

🇮🇪Ireland markconroy

Closing this individual issue, in favour of just one credit on the aggregated issue: 📌 Localgov Base - fixes eslint issues across the project Active

🇮🇪Ireland markconroy

We have also worked on the "Stable is deprecated" issue in Add support for Drupal 11 Active where we set Stable9 as the theme, so we are not relying on a contrib theme, and also updating from core to core_version_requirement.

For that reason, I think this issue could probably be closed as outdated since it doesn't reflect support for D11.

🇮🇪Ireland markconroy

Update sentence for logical properties from "do have" to "do not have"

🇮🇪Ireland markconroy

I think that MR looks good now, but maybe we should drop support for Drupal 8 + 9 since they are no longer supported.

Also, have you tested that this theme actually works in Drupal 11?

🇮🇪Ireland markconroy

Stable9 is a core theme, so that should be decent evidence for it having a future. It's the core base theme that other themes (in general) are encouraged to use as their base. It was introduced in Drupal 9, then we deprecated stable in 9.5 and moved stable to contrib (I doubt many are using it from contrib) in favour of using Stable9.

Stable9 is the default core theme for Drupal 9 + Drupal 10 + Drupal 11. You can see it's code is here in Drupal 11 - https://git.drupalcode.org/project/drupal/-/tree/11.x/core/themes/stable... - with the note:

Warning: Themes that decide to not use Stable 9 as a base theme will need
continuous maintenance as core changes, so only opt out if you are prepared to
keep track of those changes and how they affect your theme.

Here's the change record from Drupal 9.5 saying to use Stable9 instead of Stable if possible - https://www.drupal.org/node/3309392

Here's the Drupal (sub) theming guide - https://www.drupal.org/docs/develop/theming-drupal/sub-theming-using-sta...

🇮🇪Ireland markconroy

Why are you requiring the contrib version of the Stable theme rather than core's version?

🇮🇪Ireland markconroy

@Rajan Kumar@2026 your MR will not work, as it stipulates that stable is the base theme, but that is not available in Drupal 11.

You will need to update the core_version_requirement and also change stable to stable9.

I'd also suggest, the core_version_requirement removes support for 8 and 9, but that is up to the maintainer to decide.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

The free text filter on the "Extend" page is not a search component.

Please do not work on that as part of this issue.

This issue is to standardise how search will look where search is actually used when it's a single input+button in Drupal 7.

I'm not even sure there are examples of this in Drupal 10/11 core at this stage.

The only thing I can find is this:

With all that being the case, I am going to close this issue. If someone disagrees, please open a new issue with very clear instructions on what you want to achieve.

🇮🇪Ireland markconroy

Giving attribution

---
Thanks to Big Blue Door for sponsoring my time to work on this.

🇮🇪Ireland markconroy

From MDN

The :dir() pseudo-class uses only the semantic value of the directionality, i.e., the one defined in the document itself.

Looks like it's document-level.

🇮🇪Ireland markconroy

:dir(rtl) should work perfectly fine for us.

It's a CSS function that has been supported for quite a while.

https://developer.mozilla.org/en-US/docs/Web/CSS/:dir

🇮🇪Ireland markconroy

Marking as 'Needs review'

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Setting to Needs review.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Setting to Needs review so the bots can run.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Closing this issue now.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Merged, thanks for working on this.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Merged, thanks very much for working on this.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

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

🇮🇪Ireland markconroy

I agree with @finnsky. Let's do it here for Umami.

We can see how it fares and remove it when the generic version is ready, while also using our version of it as a testing ground.

🇮🇪Ireland markconroy

I'm happy to go with @finnsky's approach, looks very clean and will scale for other uses too.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

I'm happy with this so. Let's RTBC.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Tested on Drupal 11, and is working as expected with this patch.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

MR created to add Drupal 11 support.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Issue summary updated, moving back to 'Needs review'.

---

Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

Opening again so we can add more issues/child issues if we need to.

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

This was an issue in D8/9 from the Quickedit module.

Let's mark it as outdated.

🇮🇪Ireland markconroy

Thanks @finnsky

I've updated the MR now to use hook_preprocess_layout and added a new library to theme this.

Navigation collapsed:

Navigation expanded:

---
Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

@finnsky this looks very interesting.

I wonder should we do it in 2 stages: get the original JS template MR merged, and then follow-up with a proposal to use Web Components?

---

Thanks to The Confident for sponsoring my time to work on this.

🇮🇪Ireland markconroy

MR started - currently just adds the HTML to the block.

I'll try get to the CSS next week.

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

Adding patch for latest version of preview link module.

---
Thanks to Big Blue Door for sponsoring my time to work on this.

🇮🇪Ireland markconroy

I'd recommend keeping the permissions and the new designs into 2 separated issues so one doesn't block the other.

@ckrina in terms of moving this along quickly, how about we merge this MR "as is" so we have the new navigation module and we also have the "demo purposes" notification. And next week I'll start working on the 📌 Integrate Umami message Active (which I hadn't realised existed until now, sorry).

Or would you prefer me to revert the commits that move the message? If so, it will mean we have tests involving the toolbar even though we don't have toolbar installed any more.

I prefer the first option - get this merged, then do the follow-up.

🇮🇪Ireland markconroy

And (hopefully) finally: Updated the CSS so there is less of it, it's easier to read and we have the same CSS for small screens and large screens

Small screen

Large screen

🇮🇪Ireland markconroy
  1. Remove toolbar from modules to enable in demo_umami.info.yml
  2. Add navigation to modules to enable in demo_umami.info.yml
  3. Move "This site is intended for demonstration purposes" message from toobar to page_top
  4. Rename 'toolbar-warning' library and classes to 'demo-profile-warning' and classes
  5. Remove 'access toolbar' from permissions in testDemonstrationWarningMessage test
  6. Update permissions for author and editor

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

🇮🇪Ireland markconroy

Thanks for your continued work @pooja_sharma, but we can't uninstall navigation and install toolbar. If we do that we are back to the exact same state that Drupal core is currently in. The point of this issue to to install navigation and to stop using toolbar.

I'll revert your commits.

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

🇮🇪Ireland markconroy

@pooja_sharma thanks for working on this. I think instead of removing that test, we should have a clause in it that uninstalls the navigation module and enables the toolbar module instead.

Or else we'll need a follow-up issue to place that message somewhere else and update the test to reflect the new position of it.

🇮🇪Ireland markconroy

@smustgrave can you rephrase this please? I'm not sure I follow what you mean.

Think additional resource have to be up the OpenTelemtry tests?

🇮🇪Ireland markconroy

Thanks @RandalV. I'll click around a bit more and see if I can reliably reproduce the issue. Like I said in the summary I'm not even sure how/why it's happening.

Thanks for the quick response.

🇮🇪Ireland markconroy

Dammit. There's not such thing as a quick win in Drupal!

Ready for review again.

🇮🇪Ireland markconroy

Toolbar removed.

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

🇮🇪Ireland markconroy

We should account for more space then just 2 active languages. As often sites can have 4-7

I was thinking that as well, though in this specific instance we know we only have 2. But, good point. I'll make a small change tomorrow to give us the exact same experience on mobile and desktop, with all the languages in a vertical list. That will actually be less code and more scalable.

Are you concerned with JS degrade, or just the breakpoint hide/showing.

I have tested this with JS turned off, and it works fine. This is just to make sure that we don't see the button if JS doesn't load, but we do see the links.

🇮🇪Ireland markconroy

Here's a new rewrite of adding an accessible dropdown for the language switcher, which I think is a lot simpler than what we have been trying up to now. Given that no one has worked on this in over 4 years, I think a re-write is warranted, so we can stop the discussion about things not working in IE11, and compiling our ES6 code to ... etc.

Here's my approach:

  • Create a template for the language switcher block
  • Put a button in that block with the hidden attribute on it, so it's not visible on the page
  • Use JS to remove that hidden attribute so we can see the button
  • Use JS to add a hidden attribute to the languages so they are now invisible
  • Add an eventListener for a click on the button, if clicked toggle its aria-expanded attribute between true/false, then toggle the hidden attribute on the languages correspondingly
  • Add a resize function to check the window width
  • When the window is resized, if it's below768px do the above items, if not, reset everything
  • Update the CSS so we have the following screenshorts - small screen closed languages, small screen open languages, large screen languages

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

🇮🇪Ireland markconroy

markconroy changed the visibility of the branch 3042417-testing-ignore-this-branch to hidden.

🇮🇪Ireland markconroy

markconroy changed the visibility of the branch 3042417-accessible-dropdown-for to hidden.

🇮🇪Ireland markconroy

Thanks very much @opdavies. Sorry it took me until now to get around to merging it, I thought I had done so waaay back.

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

Production build 0.71.5 2024