🇮🇪Ireland @markconroy

smustgrave → credited 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.

: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

Forcing bots to run again.

Marking as 'Needs review'

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

markconroy → created an issue.

Merging.

Setting to Needs review.

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

markconroy → created an issue.

Merged.

Setting to Needs review so the bots can run.

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

markconroy → created an issue.

Closing, will open new issue.

markconroy → created an issue.

Closing this issue now.

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

Merged, thanks for working on this.

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

Merged, thanks very much for working on this.

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

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

breidert → credited 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.

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.

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

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

markconroy → created an issue.

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

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

MR created to add Drupal 11 support.

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

markconroy → created an issue.

markconroy → created an issue.

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

---

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

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.

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

Let's mark it as outdated.

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.

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

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.

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

Adding patch for latest version of preview link module.

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

nod_ → credited 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 Needs review (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.

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

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.

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.

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

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

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.

Adding credit to Open Code for the time to file this issue and work on it.

markconroy → created an issue.

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

Ready for review again.

Toolbar removed.

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

Assigning credits.

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.

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.

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

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

markconroy → created an issue.

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.

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

Fixed via a different merge request.

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

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

Thanks @joelseguin I have this merged now.

I think this approach is the correct approach, so there is a double layer before you can edit things

- it must be in the path you are allowed edit and
- you must have the correct permissions to edit it

This means if you have a node of type event inside this path but are not allowed to edit events, we catch that.

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.

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.

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.

Merge request for this issue: https://git.drupalcode.org/project/drupal/-/merge_requests/8929/diffs

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

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

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.

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.

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.

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.

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.

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.

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.

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.

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.

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.

I'm working on this.

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

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

Thanks @needs-review-queue-bot

Fixed, and setting back to needs review.

Setting to needs review.

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.

markconroy → created an issue.

Closing, as no longer relevant.

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

Closing, as no longer relevant.

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

Closing, as no longer relevant.

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

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

Updating to add credit to Code Enigma.

markconroy → created an issue.