🇨🇦Canada @joelpittet

Thanks for your help ensuring I didn't break things with this refactor @ramil g

@finnsky Right, re #94 we’re on the same page: aria-labelledby and aria-label give equivalent results, so it’s good to confirm the principle.

The snag is in #97: with the current MR the block title is gone entirely. In navigation_menu.html.twig we dropped both the <nav aria-label="{{ title }}"> and the hidden <h4>{{ title }}</h4> so nothing else outputs the name. That leaves a single sidebar landmark full of unlabeled lists. I looped @katannshaw as she mentioned this earlier up and we really do need to surface that label somewhere... either by restoring the hidden heading or wiring the title into an aria-label/aria-labelledby — so each navigation block remains discoverable. Two landmarks is fine, but we still need names for the menus inside them.

In a (hot) minute I will do a test locally with the instructions for @katannshaw mentioned above as it clarifies what I couldn't sort out of what

VO means for keyboard shortcut
so VO + F1 means Control + Option + F1

That was throwing me when I tried to test this out the first time, I couldn't figure out VO meant.

Again, I am not an expert, but I also don't want to lose valuable information for those who need it, so if you can prove it's not needed, I’d be happy to hear that advice too. Just nobody mentioned the reason behind dropping it, from my scan of the comments.

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

Thanks for the twig coding standard fixes, sorry I can't mark them as resolved in gitlab @grimreaper.

@catch Thanks for taking a stab at figuring out why the menu title/label was dropped!

I suspect that’s exactly why the aria-label/heading disappeared, but the result is that each navigation block now renders as an unlabeled list “Shortcuts”, “Content”, “Create”, etc. never reach the DOM when the block label is hidden in config. We still need an accessible name for each block even if we drop the redundant , so I’d like to restore a hidden heading or similar so screen readers can differentiate those sections again.

I would like to end in that I am not an a11y expert nor do I use a screen reader, so feel free to overrule me if someone on this thread is, I just don't want things to unintentionally get worse for those who need it. I'll try to reach out to @katannshaw in Slack to get a take as that was very well described in #31.

@katannshaw in #3452724-31: Navigation side bar and top bar should have appropriate aria labels → #31 laid out that the title should persist very well and it looked like someone added it back as aria-label in https://git.drupalcode.org/project/drupal/-/merge_requests/11122/diffs?c... I didn't see where it got dropped again but just wanted to raise that is is gone and want to ensure that is intentional and not incidental.

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

Closing as there has been no update in 11 months from the questions in #4. Feel free to reply if there are more details or create a new issue with more clear steps to reproduce.

I am about to release a fix for repeat rules Closing this as fixed

This was fixed thanks @stevewilson for RTBCing this Nov 26, 2023 4bb4c259

Thanks again @ramil g and @franceslui for getting this sorted out. I will make a new release in a moment.

Thanks for following up @4kant and I am glad you got something working.

It's amazing to have Müller, I hope I get to see him play soon.

I hope to cut another beta shortly, maybe even today.

Tests working again

As maintainer I have been making steps to getting 8.x-1.x working for moving from D7. Is that the version you are all testing, and have you tried the dev branch? If you are referring to 8.x-1.x , can you update the version for this issue?

As maintainer I have been making steps to getting 8.x-1.x working for moving from D7. Is that the version you are all testing, and have you tried the dev branch? If you are referring to 8.x-1.x , can you update the version for this issue?

As maintainer I have been making steps to getting 8.x-1.x working for moving from D7. Is that the version you are all testing, and have you tried the dev branch? If you are referring to 8.x-1.x , can you update the version for this issue?

Thanks I am committing the changes. Looks good to give the args more context from the view/display.

Thanks @franceslui I have committed to the dev branch.

joelpittet → created an issue.

Fixed and backported to 3.8.x

@franceslui, I agree with your proposal to remove it. And considering it's optional, there is no clear commit around it's introduction, and D7 didn't use it, that seems like the right move. Can you create an MR and make that change, I'd be happy to review and commit that.

@konot, are you assigned to tackle implementing this? Thanks for creating this nevertheless! I ask because I tend to no use that field as people forget they are assigned, life happens, and it holds it up from progressing the issue.

joelpittet → created an issue.

💯 with @kristen pol. I am maintaining and jumping around and missing notification emails left and right. I use the user dashboard as a “catch-up" area to look at the most recent stuff — when I have time and declare bankruptcy on the rest!

Even if you can track the save button was clicked as a "flag" to the contribution was acted upon, and create a report on the un-acted upon, I think that would be immensely helpful as I can treat it like a catch-up list — again when I have a moment free.

@franceslui RE #6 Ah, that is clearly related, it split on the comma separated EXDATE values as if they were part of the same set.

Thanks for the explanation about the semicolon split!

joelpittet → created an issue.

RE #4 @franceslui, I appreciate the details and I’m inclined to agree that WKST=Array may be related, but I didn’t see how you arrived at that conclusion from the comment. Could you share the steps you used to determine the linkage vs. WKST=Array being separate issues? For example, did you remove the EXDATE/RDATE content from the rrule column and retest to see whether WKST is output correct or something like that?

You also mentioned the current MR !10 doesn’t fix the problem. I agree: as it stands it appears to emit EXDATE/RDATE on their own lines, but it doesn’t remove those values from the RRULE line.

I’ll get it back up to working (or perish trying)

joelpittet → created an issue.

joelpittet → created an issue.

joelpittet → created an issue.

joelpittet → changed the visibility of the branch 8.x-1.x to hidden.

Committed these on the 8.x-1.x directly by accident, here's the commit messages.

commit b1743b568fd8a34ef97448347c39ac31a7bf8606 (HEAD -> 3546011-replace-deprecated-node_type_get_names, calendar-3546011/3546011-replace-deprecated-node_type_get_names)
Author: Joël Pittet <pittet@cs.ubc.ca>
Date:   Wed Sep 10 09:32:54 2025 -0700

    Fix node_type_get_names() deprecation

commit 8460b451f22bef9a8460c67dffcb220cdebd45a4
Author: Joël Pittet <pittet@cs.ubc.ca>
Date:   Wed Sep 10 09:30:11 2025 -0700

    Update PHPstan and don't allow failures

joelpittet → created an issue.

joelpittet → created an issue.

Thanks, this one was a doozy, I have committed it to 2.x

Thanks a bunch @phenaproxima created that follow-up 📌 Refactor form to use getEditableConfigNames() and $this->config() instead of injecting config Active

joelpittet → created an issue.

joelpittet → created an issue. See original summary → .

Merging this in to fix the brittle tests. We can adjust the CONTRIBUTING.md later.

Oh what is the ruleset you don't have? You might be able to add it to your composer global if you can pick it out of the require-dev dependencies.

@maskedjellybean RE #20 I have been trying to get DDEV contrib to help make this question not a thing, but there is documentation here that is generic enough that should get you over the hump:
https://www.drupal.org/node/1419988 →

I will first try to get it into date_recur 📌 Add DDEV Drupal Contrib for ease-of-maintenance Needs review , then do the same thing here. Please do have a peek at that issue, especially if you are familiar with DDEV already for your local environment.

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

joelpittet → changed the visibility of the branch 3501637-add-a-group-index to hidden.

joelpittet → changed the visibility of the branch 2325899-10.3.x-refactor to hidden.

I rerolled, diffed the diffs after — no changes other than context lines (and index hashes). And diffed the MR 3751 against #213 🐛 UI fatal caused by views argument handlers no longer can provide their own default argument handling Needs work

Trying to help this along... next I plan to review the unresolved comments, so leaving as Needs Work.

I will hide the other MRs to keep the focus on that one.

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

Forgot to change to needs review after adding the tests and attempt to shore this up by moving the logic onto the plugins.

Thanks again for digging into this @ramil g.

Thanks, @ramil g. It sounds like a good point to allow other implementations—aka, let the wrapper wrap instead of always guessing. Marking fixed and pushed to the dev branch.

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

Nice 🚀and thanks for exploring that nuance with me @albeorte

Oh and please do try 8.x-1.x branch, I need to cut a new beta soon but there are lots of improvements in there (and tests to back them up now).

Thanks for reaching out, and I totally understand the frustration—this module can be a bit tricky to get set up right now, especially coming from a working Drupal 7 setup.

We’re currently maintaining two streams of work: a port of the 7.x-3.x/8.x-1.x branch (which I’m primarily focusing on), and some experimental ideas in the 8.x-2.x branch (I am less familiar with this, and it seems to have stalled... for now). The 8.x-1.x branch is where most of the stable work is happening, but it’s still in beta and we’re actively working through a number of issues.

Because of this ongoing development, documentation hasn’t yet caught up—though you’re absolutely right that we need it, and it’s moving up the priority list as more people start using the module on Drupal 10. Maybe you can help get the ball rolling with us, porting the D7 docs?

In the meantime, if you’d be willing to share a bit more about how your view is configured, I’d be happy to help troubleshoot the current issues—especially the missing month/pager behaviour and events not displaying. That might also help us shape the documentation better around real-world use cases like yours.

Sorry, I probably conflated a couple issues, sorry! You're facing duplicate results, I was facing timezone related errors when I turned aggregation on... if you aren't seeing errors, we don't have the same issue.

I will change this to a support request until we can figure out if the problem is coming from this module or not.

Do you think you could reproduce this issue without paragraph being involved?

If you can share your views config yaml and/or SQL output I might be able to help further, or any other info to help you narrow down the problem. It seems very broad at the moment.

The tests look great, needs a re-roll of course but better to move this to a MR.

smustgrave → credited joelpittet → .

All the @todos this removes are still there. Also my question (and joke) in #27 still stands.

Needs to moved to MR

I think this is related to another issue where when you turn on aggregation, the timezone is not included in the Group fields, or at least that is a workaround.

I plan on taking a deeper look at this field handler plug-in but could you check to see if adding timezone works? Or give more details on the problem with your facing?

Nice to have options :)

“An escalator can never break: it can only become stairs. You should never see an Escalator Temporarily Out Of Order sign, just Escalator Temporarily Stairs. Sorry for the convenience.” — Mitch Hedberg

Got the tests for entity_test which doesn't implement EntityChangedInterface. I hope that gets this further. Removing the tag I added earlier.

Needs more info, can you share your menu block configuration, screenshot or words works well. And the basic structure of your menu so I can reproduce the issue?

Moving to a support request until we can prove a bug 🐛

@ressa so true! I just happened to get this in today https://github.com/ddev/ddev-drupal-contrib/pull/148

@dewancodes Drupal 11.2.3 has 🐛 Assets paths in CSS no longer rewritten when aggregation is enabled Active in it.

Can you confirm you are using that version? Also could you give more details on the asset causing this issue for you?

@rishi kulshreshtha I agree, but that will involve replacing the core date arguments, not an easy feat but I am thinking in the same direction as you.

🐛 stylelint task should create default prettier config Active was messing with me, the change is great, but locally it didn't magically add .prettierrc.json so it gave the same bad formatting advice.

We could put guard rails in like if drush is not installed say maybe you haven't run

ddev poser
ddev symlink-project

ddev symlink-project is supposed to run automatically on start, but I feel it doesn't, but all it does is symlink the project into web/modules/custom

Adding PHP 8.4 as it needs that to fail.

joelpittet → created an issue.

joelpittet → created an issue.

Thank you @hosterholz that is a really good explanation in the issue summary.

Thanks @ramil g, I will merge this in right away.

joelpittet → created an issue.

Creating a follow-up because I am getting this error as well and #2 isn't working.

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

joelpittet → created an issue.

The code looks great I should add, I am mostly concerned about the test

Thanks @joseph.olstad I will reviewMR 121 later, first glance the test could use something a bit more explicit, but I will look closer as there is probably context lines that make it clearer already in place.

Let’s make this simple, which MR only does what the issue summary and title asks of it and doesn’t creep on scope?

There are quite a few proposed solutions here, and it’s not clear which one is meant to move forward. I hid two that looked like accidental MRs (hopefully correctly!).

The issue summary is excellent though! Short, clear, and the title is specific. That really helps. Let’s make it easy for maintainers to commit something.

@joseph.olstad instead of me trying to guess, maybe you could hide any MRs/branches we no longer need? If you’re combining approaches, a patch might be a better fit anyway, since it’s composer-patchable and avoids branch clutter.

I think most of this confusion came from creating a 2.0.x target branch on a fork that didn’t already have it — GitLab makes you create it before you can select it as the MR target, which unintentionally makes it look like it’s part of the issue work. (I still haven’t figured out how to avoid that either.)

joelpittet → changed the visibility of the branch 3334240-use-get-method-2 to hidden.

joelpittet → changed the visibility of the branch 2.0.x to hidden.

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