Problem/Motivation
We are in the process of deprecating jQuery UI in core. jQueryUI dialog is among the components that need to be removed/replaced.
As mentioned in the parent issue: #3067261: [Plan] Remove jQuery UI components used by Drupal core and replace with a set of supported solutions
The OpenJS Foundation lists jQuery UI as an Emeritus project in https://openjsf.org/projects/ which is described as:
Emeritus projects are those which the maintainers feel have reached or are nearing end-of-life
This issue was originally proposed as specifically looking for a polyfill, with the following explanation:
dialog.js always aimed at using the HTML5 dialog spec. Chrome just added experimental support for dialogs in it's dev branch: http://demo.agektmr.com/dialog/
As time has moved on, it looks like a polyfill is not necessary.
We should let chrome use native dialogs when needed, it actually solves a bunch of messy issues, two of which are:
#2072037: Drupal dialog modal background z-index is set too low to reliably occlude core UI components
#1836392: In the Views UI, the interaction pattern of “All displays”/ “Override this display” is confusing
The problems mentioned above should absolutely be considered wile investigating what to use in place of jQueryUI dialog, but the options should be expanded to include different libraries - whatever option can best facilitate removing jQuery UI.
Proposed resolution
Add a native dialog element to the render API.
Add Umami/Claro/Olivero theme support
Convert existing core usages of jQuery UI dialog to use the native dialog.
Deprecate the jQuery UI dialog for removal in Drupal 11 or 12.
Remaining tasks
List of libraries assessed prior to the decision to use native dialogs:
If we are fortunate enough to find multiple libraries that meet these requirements, they should be compared using criteria such as: Code Style, Maturity, Responsiveness, Accessibility, UX, UI, Modularity, etc.
Libraries reviewed so far:
Tingle - https://robinparisi.github.io/tingle/Missing requirement #2Micromodal https://micromodal.now.sh/Missing requirement #2Jquery Modal https://github.com/kylefox/jquery-modal : modal onlyMissing requirement #2Vex https://github.com/HubSpot/vex : modal onlyMissing requirement #2Rmodal - https://rmodal.js.org/ : modal onlyMissing requirement #2Van11y dialog https://van11y.net/downloads/modal/demo/index.html - no way to open/close via JS, it's all tied to input elements.Missing requirement #1a11y dialog http://edenspiekermann.github.io/a11y-dialog/example/Missing requirement #1- Nyro modal (jquery) http://nyromodal.nyrodev.com/ : Meets the requirements but hasn't been updated since 2014.
- JqModal http://jquery.iceburg.net/jqModal/#where : Meets the requirements and even has a companion resize library. Hasn't been updated since 2016
Deque Cauldron pattern library - https://github.com/dequelabs/pattern-library. (Demos at https://pattern-library.dequelabs.com/composites/modals).Missing requirements #1 and #2. Unsure about #3. Reviewed in #26.- sweetalert2 https://sweetalert2.github.io/: Needs review
.
Libraries to review
- GoogleChrome/dialog-polyfill. Initial review in #12, needs another review to answer all the requirements.
- repo - https://github.com/GoogleChrome/dialog-polyfill
- demo page - http://demo.agektmr.com/dialog/
Implement the library
Add additional tests for important dialog use cases that aren't part of existing coverage. Before doing this review Layout Builder's test coverage as it indirectly tests a wide variety of Dialog use cases.
User interface changes
Yes, TBD.
API changes
TBD.
Data model changes
Release notes snippet
Attached is the patched used to make the following work (minus the scrolling element, that's hardcoded CSS).
What needs to be done is making sure our API can handle native dialogs when available and that pretty much means we need to not use jQuery UI or in a very different way than today to avoid wildly different UX between native and polyfill.
Comment | File | Size | Author |
---|
Issue fork drupal-2158943
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
Wim LeersAll existing jQuery UI Dialog JS plus its dependencies:
For a total of 133,831 bytes or 130.7 KiB.
Then there's the CSS too:
For a total of 24040 bytes or 23.5 KiB.
That's >150 KiB … for showing a dialog! Note that almost nothing in Drupal core uses jQuery UI, so it's extremely likely that we'd indeed be loading >150 KiB just for showing a dialog. Even if we'd be using it on all pages, 150 KiB is ridiculous. It's not acceptable, performance-wise.
(And note that I'm only looking at the jQuery UI dependencies, there's also the dependencies on jQuery, drupal.js, debounce.js and displace.js, which I'm not even counting because 1) most of those would be loaded already anyway, 2) most of those we'd probably continue to need after implementing nod_'s proposal.)
Comment #2
Wim LeersThe current patch is incomplete though: it doesn't yet use the polyfill.
We also must ensure that we don't regress in terms of accessibility.
Comment #3
nod_As the polyfill is very new and as I am very lazy, I haven't tried to get it used.
As you said, there are outstanding a11y issues and we'll need to deal with the reduced feature set of the native and polyfill implementation compared to jQuery UI.
I'm not too worried about either, there is already a PR to fix a11y issues on the polyfill and we haven't gone mad with dialog features. We should let it sit a while see how much momentum the polyfill will get.
@WimLeers: hopefully that issue will make all your pain with dialogs worth it ;)
Comment #4
Wim LeersAddendum for #1: that 150 KiB is uncompressed, of course. Typically that would compress to 25–33% of the original size. That's still ridiculously much.
Comment #11
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedTagging, I don't think I've tried this library yet.
There are numerous other dialog libraries we can consider too. There's one called a11y-dialog, which I was impressed by, and it uses the HTML5 dialig element if it detects support.
Comment #12
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedI tested the demo at https://demo.agektmr.com/dialog/ using Chrome 74 and Firefox 66. With Firefox, the page displays a message that the polyfill mode is being used.
I noticed a difference in the escape key behaviour, compared to jQueryUI dialog demos at https://jqueryui.com/dialog/
jQueryUI dialogs can be dismissed with the escape key, regardless of whether they are invoked with the
modal: true
option, so long as focus is within the dialog.The GoogleChrome/dialog-polyfill demo differs from this. Pressing escape only dismisses dialogs which have been opened with
showModal()
; it doesn't dismiss the dialog which was opened withshow()
.As I read the HTML5.2 rec., user-agents aren't required to provide the escape key behaviour at all. It's a suggested behaviour ("may") for dialogs on the "pending dialog stack", which as far as I can tell only means dialogs which have been opened using
showModal()
. The HTML5.2 rec doesn't mention the pending dialog stack in the steps for theshow()
method. So it looks like the escape key behaviour in GoogleChrome/dialog-polyfill is comformant with the HTML 5.2 rec.However it is different from the behaviour we currently get from jQueryUI.dialog, so we may risk an accessibility regression from what we have now. Non-modal dialogs are used in Drupal, notably the token browser dialog from contrib. I think all our dialogs currently have a close button at the top, which would mitigate the lack of the escape key behaviour (worth confirming though). Some uses of off-canvas dialog can be considered non-modal too, in that they don't prevent mouse interaction with buttons outside the dialog.
Aside: the WAI-ARIA Authoring Practices recommend the escape key behaviour for modal dialogs. Note that non-modal dialogs aren't mentioned there at all; earlier drafts did include them, but they were removed because there were some issues which didn't get consensus in the WAI community. I don't recall what it said about the escape key behaviour for non-modal dialogs. FWIW, my view is that when focus is inside a non-modal dialog, the escape key should dismiss the dialog.
Leaving the needs accessibility review tag in place for now. There are other dialog polyfill libraries which can be reviewed.
Comment #13
Wim Leers#11 + #12: hah, such a timely comment, because a few hours before you posted #11, I moved #3037446: Forms with required fields marked by asterisk do not have text explaining what the asterisk means to the core issue queue.
Comment #14
nod_The initial discussion was 5 years ago, I'm sure there are better libs out there by now, I wouldn't get too attached to the initial lib suggestion.
Comment #15
Wim LeersLet's see if somebody else is interested in helping out with this: https://twitter.com/wimleers/status/1157261418913308672
Comment #16
bnjmnmComment #17
bnjmnmSome findings on a11y-dialog from #3071552: Replace jQuery UI dialog with a supported dialog library
<summary>
is not considered a focusable element - also an issue with jQueryUI. Also missing<embed>
(it is confusing because there cab be focusable<details>
without<summary>
, but as long as<summary>
is listed as a focusable element, focus will be properly trapped within<details>
, even if it does not have summary.Doesn't look like there's a mechanism to elegantly pass in configuration options that would make it possible to have different styles/types of dialogs and settings like height and width. I was able to kind of implement this behavior by removing the creation ofI found additional evidence that suggests this is possible - but some of the other issues still make this a not-ideal option.#drupal-modal-wrapper
on init (which probably introduces other problems), so the attributes of the triggering element can be passed on to attributes in the dynamically created dialogDrupal.AjaxCommands.prototype.openDialog
so dialogs can be styled differently.Comment #18
bnjmnmI'm providing a patch that's more work-in-progress than I'd typically upload. Seeing how the jQueryUI issues are moving quickly and I'll be offline for at least a week, I wanted to make sure this was available so no work was duplicated.
The attached patch uses a native dialog when available. Browsers without native dialogs currently get a console.log().
Most of the work done was replicating the behavior of jQueryUI, so anything defined in a PHP render array or config yml will result in the same dialog. This patch already has Layout Builder off-canvas dialogs working near-identically to jQueryUI. Modals also largely work, but positioning hasn't been implemented yet. It'll still take some work (resize hasn't been implemented at all yet...), but replacing jQueryUI dialog is more feasible than I initially thought. Since dialog now has to work with native and a library equally, it's easier to swap libraries if needed.
It's still necessary to find a library to support browsers that don't have native dialog. The two major requirements of the library are:
Although it would be nice, I the library doesn't necessarily need to be a11y focused - providing the accessibility capabilities via Drupal should not require much additional effort, and some of this was necessary to accommodate native dialogs. Here's some of the libraries reviewed so far
Nopes:
Tingle - https://robinparisi.github.io/tingle/ : modal only
Micromodal https://micromodal.now.sh/ : modal only
Jquery Modal https://github.com/kylefox/jquery-modal : modal only
Vex https://github.com/HubSpot/vex : modal only
Rmodal - https://rmodal.js.org/ : modal only
Van11y dialog https://van11y.net/downloads/modal/demo/index.html - no way to open/close via JS, it's all tied to input elements.
Maybes
Nyro modal (jquery) http://nyromodal.nyrodev.com/ : Meets the requirements but hasn't been updated since 2014.
JqModal http://jquery.iceburg.net/jqModal/#where : Meets the requirements and even has a companion resize library. Hasn't been updated since 2016
Comment #19
Wim LeersWhere in Drupal core do we use resizable dialogs? I don't think I've ever seen that!
🥳
Comment #20
bnjmnm#19
The off-canvas dialogs in Layout Builder are resizable and I've seen evidence that users make use of that resizability. I suspect that's the case for other uses of off-canvas but that is the one I know for sure.
Comment #21
Wim LeersThanks! TIL 🙂
Comment #22
bnjmnmComment #23
Martijn de WitI believe if a library misses requirement #1. It will always be possible to add that feature. I don't believe this is a big feature request.
Shouldn't accessibility be also a major requirement in usage?
Comment #24
bnjmnm#23These are good questions and I'm glad they're being asked early in the process!
#23.1
This functionality is currently part of jQueryUI dialog and is key to how the
dialog:beforecreate
anddialog:aftercreate
events work. If there's a way to preserve these events without the ability to trigger open/close via JS, I am very interested in knowing as it would make things easier.This functionality also means that native dialogs and library-provided dialogs can largely use the same set of JavaScript. It's a dozen lines of additional code vs. potentially hundreds.
#23.2
Having the dialog be accessible is very important. It's not listed as a must have of the library as it's not particularly difficult to fill in accessibility gaps on the Drupal end (in fact, Drupal does a better job of constraining focus than even the accessibility-focused libraries) If there are multiple options that meet the must-have requirements, the more accessible option will certainly be chosen.
Comment #25
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedCan someone explain requirement #1, preferably with an example of where this behaviour is already being used, or a use-case where we might want to use this in future?
Dialogs open as a result of a user interaction. If they open without a user interaction, that's likely to be a failure of WCAG SC 3.2 5 "Change on request". The reason is that focus moves to a dialog when it opens, and that counts as a "change of context" for purposes of WCAG. If we have any instances of dialogs appearing without being a direct result of a specific user interaction, we should review those.
I think that requirement #1 is about situations where a user action may OR may not need a follow-on action from the user, depending on business logic. For example there are issues which propose using dialogs for the confirmation step in entity deletion operations (currently these are a redirection to a confirmation page). Have I understood this correctly?
UPDATED: Consider a situation like this...
This situation would pass WCAG "Change on request" IMO. The dialog appeared as clear consequence of a user pressing a button, even though the dialog was instantiated using requirement #1, and the decision to display the dialog was governed by business logic.
Note that messages which pop up as notifications are not dialogs, and shouldn't use a dialog element or role.
Comment #26
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedHow many non-modal dialogs does core have?
I'm aware there are used in contrib. Notably, the token module uses one for the "browse available tokens" feature.
If requirement #2 is a sticking point, I suppose there's no hard requirement that modal and non-modal dialogs have to use the same underlying library?
I suspect the reason we're not finding much support for non-modal dialogs, is that they are actually very rare in web applications. I've often spoken with web developers who have never even heard of the concept of a non-modal dialog. Even among desktop applications, they are pretty uncommon, and are sometimes conflated with concepts like "dockable/floating toolbars", "palette", or "inspector".
Non-modal dialogs in web applications have been rather problematic for accessibility. A non-modal dialog pattern was included in an early draft of the ARIA Authoring Practices, but it was removed due to numerous problems which couldn't find a consensus. The main sticking point was how to locate them, when you aren't inside one. Dialogs aren't landmark regions, and are not required to have headings (ours do). In desktop applications, dialogs are easier to locate, since they are real windows. (Non-modal dialogs sometimes use a special palette/inspector window type with a standard keyboard command to locate them.)
Personally, I think it's worth reviewing our use of non-modal dialogs. Custom landmark regions may be more appropriate. Still, that doesn't help us with the current problem of replacing the jQuery UI library in a backwards-compatible way.
The list of libraries tested added to the summary in #22 overlooked GoogleChrome/dialog-polyfill which I reviewed in #12. I recall I was reasonably impressed with it at the time, but it could do with a more thorough API review perhaps.
I've also taken a look at the dialogs from dequelabs/pattern-library. It only aims to address modal dialogs by the look of it. It misses requirements 1 and 2, and I'm not sure how to evaluate requirement 3. I dug around the code to look at available methods. It's an interesting component accessibility-wise, but it doesn't attempt to be a polyfill for the HTML dialog element. It has interesting code to handle
aria-hidden
on elements outside the dialog, to prevent a screen reader finding them in browse mode.Comment #27
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedDidn't mean to remove the parent.
Comment #28
Martijn de WitWith my first remark about requirement #1 I was more aiming on requesting the listed libraries to add this feature for us.
Maybe some active libraries are willing to add this feature for us if we are going to use their library here.
Comment #29
catchJust a note to say we don't necessarily have to do a 1-1 replacement with full API backwards compatibility, we could also do something like the following if the result ends up being better or if it's the only viable option:
1. Add an entirely new library adjacent to drupal.dialog, for want of a better name drupal.dialog2
2. Deprecate the drupal.dialog and jquery.ui.dialog libraries
3. Remove drupal.dialog and jquery.ui.dialog in Drupal 9
4. Eventually (not sure when but probably when 8.x goes EOL), duplicate drupal.dialog2 back to drupal.dialog and deprecate drupal.dialog2
This still allows contributed modules to update to the new library in time for 9.0.0's release, because we've deprecated an old API and added a new one.
Comment #30
andypostbtw why it could not be the same library just with v2 in library definition?
Comment #31
bnjmnmThis is all great feedback, lets see what I can help with...
#25
I'm not aware of any instances where dialogs open without user interaction, that is a very reasonable concern. This requirement makes it possible to trigger the events
dialog:beforecreate
dialog:aftercreate
dialog:beforeclose
dialog:afterclose
, which are currently triggered anytime a dialog is opened/closed. These events are used by Ckeditor, Settings Tray, Toolbar, Layout Builder and Media Library. A common use for these is to add/remove classes on specific elements when a dialog is opened/closed and to trigger resizing.A specific example I recently worked on can be found in Layout Builder. When a dialog is open that is specific to a block or section, that block/section is highlighted so it's clear what part of the layout is being configured. This is implemented by leveraging the dialog:aftercreate event, which pulls a data attribute from the dialog that indicates the element that needs to be highlighted.
#26
This is certainly a good thing to know when gauging the importance of non-modal dialogs.
The UI for both Layout Builder and Settings Tray make extensive use of off-canvas dialogs, which are non-modal. I believe all uses of non-modal dialogs in core are via off-canvas, but I'm not 100% sure on that. If that's the case, it might be possible to implement off-canvas in a way that does not require a dialog library and (despite the overhead) worth exploring if there are no good options that meet requirement #2.
#26
Thank you for pointing this out! This will get reviewed.
#26
Thats a good point. I'm tagging this with "Needs followup" to make sure that gets opened as an issue. The refactoring necessary to get rid of jQueryUI dialog should also make that transition more feasible.
#28
Thank you for clarifying - I think it's best to keep it as a must requirement as it depends on action outside of the Drupal project, but I also think it's good this was pointed out as it could be a good angle to pursue if other libraries are lacking.
Comment #32
catchComment #33
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedOver the weekend, in a thread on the javascript Slack channel - @xjm said that if anything looked risky or maintainers weren't comfortable with it, there was a possibility of forking a jQueryUI component into Drupal core, and remove it eventually by D10.
To me, this means we could resort to using a new library for modal dialogs, but keep jquery.ui.dialog around to power non-modal dialogs, as a mid-term solution. I think that's what @catch is saying in #29.
Obviously this isn't the preferred approach, since it presumably brings an extra maintenance overhead, and may complicate the refactoring steps that needs to be done.
Any thoughts on this?
Comment #34
catchNot quite. I'm saying that we could take a decision to drop dupport for non-modal dialogs in 9.x altogether (if we want to), as long as they're deprecated with a recommendation on how to replace them in Drupal 8 first.
However to deprecate them (with no 1-1 replacement) in Drupal 8, we'd need to have both the new and old dialog implementations available in 8.x at the same time (via a new library definition or as @andypost suggests a v2 although I've not seen that working before).
Forking the library in Drupal 9 is something that's a final fallback if nothing else can happen in time for the release.
Comment #35
bnjmnmUpdated issue summary to indicate that GoogleChrome/dialog-polyfill meets the requirements and is being explored further as a possible replacement.
Comment #36
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedThanks @bnjmnm, that's good to know. I'll have another go at manual accessibility testing on the google polyfill soon.
Comment #37
bnjmnmThe attached patch eliminates use of jQueryUI dialog and uses native dialog or GoogleChrome/dialog-polyfill depending on the browser.
There's still quite a bit of work to do, but this implementation passes all tests (which cover a wide variety of dialog use cases), so things are moving along.
Drupal.tabbingManager.constrain
https://www.drupal.org/project/drupal/issues/3074267#comment-13249857
Comment #38
bnjmnmCreated an issue that surfaced from the work happening in this one: #3082602: Remove transform rule from css_disable_transitions_test
The animation-disabling in Javascript tests is necessary for dialog tests to work consistently. These tests include disabling
transform:
, which causes a problem: the positioning for these new dialogs is managed by PopperJS, which usestransform:
for placement. When animations are disabled (necessary for dialog tests), this can also result in parts of a dialog being out of viewport, and the test failing as a result.For this issue to land, either #3082602 will need to be committed, or the patch in this issue will need to add a way of making transforms work in Javascript tests while other animations are disabled.
Comment #40
xjm@catch and I discussed that in an ideal scenario, we'd add optional support for the new dialog API (whatever it may be) in a minor release, similar to what we're doing with #3076171: Provide a new library to replace jQuery UI autocomplete, and deprecate the old dialog once the new API's stabilized and tested. It's a bit less straightforward than #3076171: Provide a new library to replace jQuery UI autocomplete as the new API wouldn't have independent product value the way the new autocomplete widget will (so an experimental module a site owner turns on makes less sense), but we can probably figure something out so that we can do the replacement with less risk to stability and backwards compatibility.
Also similar to #3076171: Provide a new library to replace jQuery UI autocomplete: unlike most significant additions, the new API can be added in 8.9.x/9.0.x, because it's a critical security hardening.
Comment #41
xjmComment #43
Chi CreditAttribution: Chi commentedAdded Sweet Alert 2 to the list.
Comment #44
bnjmnmReroll for Drupal 9. This will likely also need refactoring due to the new version of PopperJS along with what is needed in #37
Comment #46
bnjmnmThis updates #44 to work with PopperJS 2.x. After the reroll/refactor this is back to passing tests and behaving pretty much like jQueryUI dialog, but bost the things that were mentioned as needing to be done earlier still need to be done. A few differences:
Comment #47
finnsky CreditAttribution: finnsky at Skilld commentedFound problem:
In claro and seven `assets/vendor/jquery.ui/themes/base/dialog.css` is overriden in info file. https://git.drupalcode.org/project/drupal/-/blob/9.0.x/core/themes/seven...
but in current approach that jquery UI themes files attached to drupal.dialog
so it partially breaks dialog layouts.
Comment #48
finnsky CreditAttribution: finnsky at Skilld commentedAlso there is a problem with horizontal positioning with toolbar left position and body with margin.
Comment #49
finnsky CreditAttribution: finnsky at Skilld commentedto #46
rerolled
1) removed ignored tests in /core/drupalci.yml
2) removed ignored css properties. already removed in core core/modules/system/tests/modules/css_disable_transitions_test/css/disable_transitions.theme.css
3) added fixes for #47 and #48
4) updated backdrop styles for claro and seven themes.
5) updated polyfill library to latest version.
Comment #51
bnjmnmThis is essentially a start-from-scratch based on steps I had to take in similar JS issues.
Unlike my prior attempt, this implementation is built to backwards compatible with all uses in core (and potentially contrib), and they are implemented in a manner that makes it possible to have the "new" dialog functionality live in an experimental module. The changes are currently in misc/dialog.es6.js so all tests run with this dialog implementation, but it can be moved to a module easily. There are a few other changes outside of dialog.es6.js, but they are ones that will also work fine with the jQueryUI implementation. Without some level of jQueryUI BC, the changes needed for dialogs (particularly off-canvas) to work from D9-D10 would be really unreasonable.
Also, my earlier patches would be near-impossible to review. This patch implements dialogs as a class so it's much easier to assess scope (it's still an enormous patch, but it should make far more sense to a reviewer).
Here are some remaining tasks and other bits of info:
Drupal.Dialog
class are mimicing what jQuery UI dialog does, to the point where it's very possible there's lingering jQuery UI cruft that can be removed. In other words, this is still kind of ugly... but it's a dialog solution that supports core BC (that includes off canvas!), takes advantage of native dialogs, and has an added feature of returning focus to the opening element when a dialog closes.Comment #52
bnjmnmReroll
Comment #53
lauriiiModals are not rendering correctly. For some reason modals are very short and there is both vertical and horizontal scroll bar.
Is this just leftover?
I'm still in process of reviewing compatibility against https://api.jqueryui.com/dialog. However, I thought it would be helpful to post what I've discovered so far. Some of this could be also addressed by adding more tests.
Comment #54
dqdSorry for the noise but I need to say that: Wow awesome work goin' on here! +1 for @bnjmnm and all others on this. Impressing how such a big woop is qoing on so well.
Exactly. 100% agree!
Pretty sure it is.
Question: SInce this is a change taking effect somewhat later, shouldn't we additionally test against PHP 7.4 and 8.0 ? (link to php versioning time graph)
Comment #55
lauriiiI made some changes to improve the positioning of native modal dialogs. I also discovered a bug where the native dialog was positioned temporarily as absolute, leading into browser scrolling to the middle of the page. I commented out the bit that is causing the problem since I wasn't sure why it was needed.
Comment #56
xjmSome code style errors in the latest patch:
Comment #57
vsujeetkumar CreditAttribution: vsujeetkumar at Srijan | A Material+ Company for Drupal India Association commentedFixing some code style errors mentioned in #56, Please have a look.
Comment #58
bnjmnmBecause it's also needed by Autocomplete's shim and is a huge task on its own, I opened #3201170: Address jQuery UI position dependency to shim jQuery UI position to Popper. Several of the positioning problems with the dialog replacement will be solved or made easier to address by this shim.
Comment #59
bnjmnmSimilar to position, the resize functionality of this shim has been moved to a child issue #3201835: Replacing(?) jQuery UI resizable.
Comment #61
nod_Comment #62
nod_Following a meeting with bnjmnm, lauriii, gabor, and myself, todo:
Comment #64
nod_rerolled for 9.3.x
Comment #66
xjmI just officially moved #3076171: Provide a new library to replace jQuery UI autocomplete to #3265680: Deprecate dependencies, libraries, modules, and themes that will be removed from Drupal 11 core based on the writeup in comment #170 of that issue. Since that means we will not be removing jQuery as a dependency of Drupal 10, and since the Dialog replacement is also likely to cause disruptions that will require a longer testing period, I'm moving this issue to the same meta.
Comment #67
catchThink that makes this 10.x-only.
Comment #68
andy-blumIf this issue is now D10 only, then we ought to be able to use the native dialog element instead of any library, correct?
Source
Note that while Opera Mini says it does not support the element, CanIUse only displays compatibility with that browser's extreme saver mode, which we don't support.
Comment #69
andy-blumCode pen for the curious: https://codepen.io/andy-blum/full/KKoBpZY
Comment #70
mherchelYep! Agree that we can (and should) use the native
<dialog>
element in D10 since Safari 16 will be released before D10.Comment #71
shaalIn https://www.scottohara.me/blog/2019/03/05/open-dialog.html (last updated: 2022.03.15), Scott O'Hara is reviewing the accessibility of native
<dialog>
.I am using
<dialog>
in all my new projects as it already works great in all modern browsers (Chrome, Firefox, Safari).(even on Firefox ESR, which is version 102)
Comment #72
bnjmnmNative dialog has been the approach as of ~3 years ago since I posted #18. It does sound like the polyfill may not ultimately be necessary because, let's face it, this is very unlikely to happen in 9.x.
The biggest hurdle is that core and contrib makes heavy use of the larger jQuery UI API such as events, draggability and resizability. There are not convenient native equivalents for those so we have to figure out ways to deprecate/BC/provide equivalents and other things to avoid breaking existing sites with the change.
There's also a markup backwards compatibility concern as a native dialog uses the
<dialog>
element, and Drupal core dialogs use<div>
and there are styles, tests and JavaScript that rely on that assumption.Comment #73
Chi CreditAttribution: Chi commentedWhy not first introduce a new dialog API based on native dialog element? We don't have to drop all jQuery UI usage in the same patch.
Comment #74
andypostNative dialog is not supported on Safari still https://caniuse.com/dialog
Comment #75
bnjmnmThat's not what I'm seeing, perhaps I'm overlooking something?
Comment #76
andypostI was wrong, so only default styling prevents the issue
Comment #77
mgiffordAdding WCAG SC 3.2 5.
Comment #78
catchfwiw two elements seems like a good approach here, then we can switch things over in core and deprecate the jQuery UI version. It would be more like 'Add a native dialog element', 'Use the native dialog element', 'Deprecate the jQuery UI dialog'.
Comment #79
larowlanLate to the party here but agree with the approach in #78, came looking for this after watching IO videos and realising we no longer need the polyfill 🎉
Comment #80
catchI've tried updating the issue summary and title, it's a bit tricky since there's a fair amount of change of direction in this issue over time.
Just to expand on #78, by providing a new element, I think we should completely ignore bc for anything jQuery UI provides, whether it's the js API or markup. We will need to figure out styling in Claro/Olivero/Umami which is probably the main bit of work this creates compared to the previous approaches.
Comment #83
TinaRey CreditAttribution: TinaRey at PreviousNext commentedI created a new fork and did a bit of work on this – it's a rough WIP but there's a few native dialogs now :) (added 2 screenshots)
- created a new library
dialogNative
with the idea to have both in parallel and switch over bit by bit- changed editor and field_ui to use the new library:
editor: on a wysiwyg field, change the text format between basic_html and full_html
field_ui: on a content type, manage fields and attempt to delete one
in both cases a native dialog should pop up and the buttons should work
Notes/questions/next steps
- The existing
Drupal.dialog.showModal()
and the nativeshowModal()
function have the same name, which gets slightly confusing.-
showModal()
andshow()
seem to be doing mostly the same thing, just one with and one without backdrop? What did I miss?- Atm I've just set
app/core/lib/Drupal/Core/Render/MainContent/DialogRenderer.php
andapp/core/lib/Drupal/Core/Render/MainContent/ModalRenderer.php
to use the new native library but if we want both in parallel, this probably needs another solution. Too backend-y for me.- Currently ajax response data is just put out again as is, this probably needs some checks.
- There's currently nothing for focus, tabbable, or the whole button change thing, I assume we need some of that back in.
- Needs tests
- Needs styling
Comment #84
Chi CreditAttribution: Chi commentedThey have different use cases. Modal dialog requires user action. It blocks interaction with other parts of the page. Also keyboard navigation is captured within a dialog.
Comment #85
nod_Very cool to see work on this again!
That was intended, sort of like there shouldn't be an intermediate API between Drupal and the dialog API.
Drupal.dialog()
should just return the nativedialog
DOM element. The return of the Drupal.dialog() function was made to mimick the native dialog DOM API. So something along the lines of the following should be enough:We can figure out events later (if needed), maybe we won't need them since we'll need to call attachbehaviors somehow on the dialog contents and contrib should not need to worry whether the content is in a modal or on a page. We had to do that before since jQuery has a lot of options that can be set, but that is not the case for native dialogs.
yep, same-ish. When openning a modal the rest of the content is not supposed to be "interactable": https://html.spec.whatwg.org/multipage/interaction.html#blocked-by-a-mod...
That's ok, just need a way to test things out for now :)
see the original
Drupal.AjaxCommands.prototype.openDialog
function. it uses the insert command to add html to the dialog so that behaviors are run automatically and all.I'd test as-is for now. Seem like the focus is handled automatically by browsers and set it back to the triggering element when closing, probably nothing too bad to fix. As for buttons as long as the code is supposed to change existing form submit buttons it should work as-is. Views UI might give some trouble but we'll see when we get there.
Comment #86
kostyashupenkoMakes sense to have a look in `inert` html attribute https://html.spec.whatwg.org/multipage/interaction.html#the-inert-attribute to improve keyboard navigation.
browser compatibility seems quite good already https://caniuse.com/?search=inert
Comment #88
finnsky CreditAttribution: finnsky at Skilld commentedHello all!
I want to propose another approach that seems reasonable to me.
One of the main problems with the current implementation of the dialog in Drupal is the strong binding to jQueryUI styles and markup. This leaves the developer with few options, such as changing the order of dialog elements or adding new elements to the markup.
Currently, browser support for Web Components is the same as Drupal. https://developer.mozilla.org/en-US/docs/Web/API/Web_components
This way we can create a web component drupal-dialog which will
1. Create events when opening and closing.
2. Have an extremely flexible template, easily customizable via Drupal.theme.dialogTemplate
3. Support custom CSS properties
4. Save element styles in dialog slots. Example with claro buttons: https://gyazo.com/b8cb363cb5854415f0b023dcbf5d7fd1
This merge request is quite crude, but it does give some insight into the future benefits of this approach.
Currently tested is the confirmationDialog for changing the text format on the node edit page. For the test, add content to restricted HTML and change the format to basic. https://gyazo.com/e7a6f36d8b8e96e0f7719efdbeddfed9
Please review, Thanks!
Comment #89
AnybodyThanks, #88 might be combined with https://caniuse.com/dialog
How can we proceed here and make the required decisions?
Comment #90
finnsky CreditAttribution: finnsky at Skilld commented@Anybody
This drupal-dialog already contains HTML dialog inside.
I think the rework order should be as follows. We have to:
1. collect all usage of https://api.jqueryui.com/dialog/ API in the core.
2. Make a list of options, events and methods that the new component should/can support.
3. Write a temporary Drupal.DialogComponent that will provide this API and gradually switch Drupal to use the new component. Rewrite tests. And so on
Comment #91
larowlanI think the way forward here is what was agreed above by nod_ - start with an implementation that is based of a native dialog element and has less features than jQueryUI, add that alongside the jquery UI one and deprecate it.
If there are missing features, we can look to implement them in follow up issues, otherwise we risk making no progress at all.
I would encourage you to open a new issue @finnsky listing the features that native dialog doesn't provide that we may need to consider once we have a working alternative.
Comment #92
finnsky CreditAttribution: finnsky at Skilld commentedAdded subtask
https://www.drupal.org/project/drupal/issues/3390549
Comment #93
finnsky CreditAttribution: finnsky at Skilld commentedAdded suggestion for replace resizeable
https://www.drupal.org/project/drupal/issues/3201835
Comment #94
cosmicdreams CreditAttribution: cosmicdreams at Velir commentedHi gang, I thought I had commented here, but it looks like I haven't.
I worked on an idea for replacing some of the behaviors of the off-canvas dialog with the Popover Api. Firefox still doesn't have a full implementation but hides it behind a flag. Lots of active development towards completing it.
https://www.loom.com/share/0dde75887f884ff3be3e4980601dccb9?sid=72085fcf...
Comment #95
mgifford@rkoller & I were talking. This advent calendar article came up You don't need JavaScript for that. There are likely other gems in this blog post, but...
Most browsers support the dialog element now. Not that this would be a trivial shift, but it does seem like this might be a good time to consider it.
Comment #96
AnybodyFor sure jQuery UI dialog becaime one of the most annoying things when handling the (on the other hand) super useful and genius
use-ajax
class functionality.This is definitely something we (the community) should push forward! Totally agree and changed the title accordingly.
Comment #97
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedOK, to summarize the current state of this issue, it seems, that we agreed on using the native "dialog" element through adding an additional js library called "dailogNative". And slowly but surely deprecated the use of the jquery "dialog" library in the future. @TinaRey created a first draft with many of the required changes implemented ( see MR !4187, great work btw!). @catch and @larowlan agree with that approach in #78 and #79 respectively, and many others suggest a similar approach.
MR !397 by @bnjmnm and @_node is quite impressive but replaces the library instead of adding a new one, which could lead to problems for contrib modules, since we do not have a proper deprecation warning in place. But, we might be able to use some of the code from it :MR !4187.
The web-component approach by @finnsky also looks very promising. But should be added on top of the native dialogue library in a follow-up issue (if I understood that correctly).
I guess we can close these two MRs then for visibility’s sake and solely focus on MR !4187.
Reposting questions by @TinaRey in comment #83
A few of these questions were already answered by @_node in #85 shortly after. So let's get to it! :)
Comment #98
cosmicdreams CreditAttribution: cosmicdreams at Velir commentedIs there a specific Slack channel where we're meeting to discuss this work?
Comment #99
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commented@cosmicdreams I don't think there is one yet! Just saw your comment in #94 didn't even though about off_canvas using jquery ui as well!
Comment #100
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedAlright everyone, so based on the work done by @TinaRey and inspired by the jquery dialog adapter:"dialog.core.es6.js" by @bnjmnm. I worked on a contrib module the last couple of days, which "bridges" all existing Drupal.dialog() calls to use the new dialogNative library, called Dialog Native. It is still very much in a very early alpha stage and needs major testing, but creating dialog links such as:
And the examples by @TinaRey:
Already work. Although stuff like the off_canvas library is unfortunately broken, so no proper layout_builder for now...
Eventually, we could work on the module together, until it is solid, and we are able to merge large parts into core! I can also give out maintainer ship to speed things up, if necessary! Also I am very much in favour of a dedicated slack channel. Switching out the native dialog element with implementations of the popup API mentioned by @cosmicdreams, shouldn't be a problem, but we should discuss, what benefits it would bring us!
Comment #101
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedI created a dedicated Slack channel, to discuss further work on Dialog Native and whether future issue work should be moved to dialog_native instead: https://drupal.slack.com/archives/C06EYQU1F98
Comment #102
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedI dm'ed @_node, @bnjmnm and @larowlan, concerning future plans on this issue and maintainer ship for https://www.drupal.org/project/dialog_native.
Comment #103
dqdJoined.
Count me in and feel free to add me.
Comment #104
cosmicdreams CreditAttribution: cosmicdreams at Velir commentedOn the horizon:
Comment #105
andypostThere's related #3296098: Removal :tabbable usage in dialog.js
which should unblock #3278625: Deprecate/remove jquery.tabbable.shim