Removal :tabbable usage in dialog.js

I think this is causing 📌 Nightwatch tests fail with "unsupported pseudo: tabbable" after jQuery 4 update Postponed and hence blocking jQuery 4, bumping to critical.

#6 is wrong, maybe the last :tabbable usage got fixed in the meantime.

pretty sure the nightwatch error on jQuery 4 is the shim itself, so we should close this in favour of removing the shim. Marking as outdated.

Nope #6, is right... MR coming up.

linting failed

Yeah I've messed something up between copying the method over/tabbable conversion/linting fixes.

It was easier for me to start from scratch. Please review

Something wrong happends with off-canvas
https://gyazo.com/b264ef37eb621d29a8ffb47807817494

This Fix doesn't look like `hot` anymore

https://jqueryui.com/upgrade-guide/1.12/#deprecated-dialogclass-in-favor...

If the class placement changes are good, the Drupal\Tests\system\Functional\Ajax\OffCanvasDialogTest test also need the same fix you already did in core/tests/Drupal/Tests/Core/Ajax/OpenOffCanvasDialogCommandTest.

catch → changed the visibility of the branch 3296098-removal-tabbable-usage to hidden.

dialogClass

is attached in ignored code:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/assets/vendor...

so probably all places with deprecated dialogClass should be rewritten(checked at least)

I've replaced dialogClass usage. We also need to deprecate it in dialog.js somehow.
Some dialogs works wierd. Good testing required here.

Down to a single fail in the pipeline in workspaces now, but it looks like workspaces is already updated in the MR?

I think we should probably go ahead here with a CR once that is fixed? The changes look fine, the main issue is notifying contrib about the dialogClass change, but breaking contrib with 11.x alphas because we updated to jQuery 4, is more notification for contrib than we currently have which is nothing. We can even grep contrib and open individual issues or open a rector/project update bot issue to look for it..

I've checked. Test is correct. Workspace module not attaches class. And broken now

I think we should commit this to unblock the jQuery 4 update, and then open a follow-up to figure out what to do with dialogClass. It seems to me that we should remove it from our 11.x fork altogether, and trigger a deprecation (if possible) when it's used in 10.3.x - but not sure how feasible that actually is between jQuery UI and our JavaScript deprecation support.

Seems @catch has done the review

Opened 📌 Trigger a JavaScript deprecation error for dialogClass in forked dialog.js Active per #25.

Will go ahead and mark.

Looked at this a bit more and the PHP-level deprecation should be straightforward, I've pushed a commit for this.

Assuming that's OK, after 11.x commit, we'll want a 10.3.x MR here with the changes to the dialog command, without any of the js changes.

I think we can keep 📌 Trigger a JavaScript deprecation error for dialogClass in forked dialog.js Active open for what to do with dialogClass in the forked dialog js itself.

The 10.3.x bc/deprecation layer is working, in that tests pass - apart from a couple of test failures due to the new deprecation being triggered.

So if we clean up the dialogClass usages in the 10.3.x branch the same as the 11.x branch, we should get to green on 10.3.x too. Waiting for a full run to finish that only has those deprecation failures before doing that though.

Just realised after doing all that, there is probably no harm in backporting the dialog.js changes to 11.x, given that the deprecation/bc layer works. So we can probably do without a 10.3.x branch here after all, and backport the entire change from 11.x to 10.3.x?

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

couple of questions in the 10.3 branch, the 11.x is good to go IMO

Posted a few comments on the 11.x MR. Otherwise this looks good to me :)

Feedback from @nod_ appears to be addressed.

Updated the CR slightly to include a before/after snippet, I find it helpful but feel free to remove if it's clutter.

nod_ → closed merge request !7370

nod_ → closed merge request !7483

Committed 66c3914 and pushed to 11.x. Thanks!

Feels weird to commit 2 different patch for 2 branches that's what it is :)

Automatically closed - issue fixed for 2 weeks with no activity.