After installing the module and setting the Links display to Modal, all modals display stacked up onscreen without being clicked prior. From what I can tell this rule in dialog.css is causing the issue:
/* Icon label. */
.add-to-calendar {
display: inline-flex;
}Having a display rule seems to interfere with the un-clicked modal display. Could it be made more specific so as to affect only the List of links display, such as:
/* Icon label. */
span.add-to-calendar {
display: inline-flex;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | addtocal-augment_after-fixes-modal.png | 142.1 KB | tirupati_singh |
| #4 | addtocal-augment_after-fixes.png | 132.42 KB | tirupati_singh |
| #4 | addtocal-augment_before-fixes.png | 142.57 KB | tirupati_singh |
| stacked-exposed-modals.png | 45.83 KB | sclsweb |
Issue fork addtocal_augment-3475608
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 #2
tirupati_singh commentedComment #4
tirupati_singh commentedHi @sclsweb, I've fixed the modal opening issue even when the calendar button has not been clicked for the
Links Display: Modal dialogformatter configuration. Before fixes, the modals were opened without clicking on the button. Now, the modals are displayed only when the button has been clicked. I'm attaching screenshots of before and after fixes for your reference. Please review the changes.Thanks!
Comment #5
sclsweb commented@tirupati_singh, I tested your changes and they seem to work; however, via merge request I proposed an additional change -- I believe the CSS to ensure that the open dialog is displayed belongs in modal.css. I was seeing a separate display bug when modals were selected but not icons -- the modal was never being shown. My change fixes this and should cover both modal scenarios (icons selected or not).
I believe the line of code,
.add-to-calendar[open],can be removed from icons.css as long as it stays in modal.css. Could you test and see if that change works for you?Comment #6
mark_fullmerAssigning myself for maintainer review and increasing the priority from "Normal" to "Major."
Comment #7
mark_fullmerThanks for the bug report. I am able to reproduce the problem on a generic Drupal installation using the
Standardinstallation profile.I agree that the root cause of the problem appears to be:
However, in my testing, at least in the context of a Drupal
standardinstallation, using the Basic Page node (i.e., not Layout Builder), simply removing that declaration resolves the issue.Before proceeding, then, I think it would be good to hear from @mandclu about what the intent of that CSS was. For example, is it to accommodate a scenario for Layout Builder? Note, too, that the most recent commit adjusted that CSS slightly: https://git.drupalcode.org/project/addtocal_augment/-/commit/687669f6478... . (Both
flexandinline-flexcause issues in the context of a basic node using the field display.)Comment #10
mandclu commentedThe original intent was to make sure that in narrow spaces, the of icons would wrap as a group, but clearly this had unintended consequences when using the modal display. Simply adding the span to the selector seemed to work for me, so adding that change as a separate MR.
It would really have been better to fix the CI tests in a separate issue, but they're pretty limited in scope so I'm not going to stand on formality there.
If the simple span change works for everyone then happy to get this fix merged in. If the additional changes using the
[open]selector are also needed, please provide steps to reproduce the use case.Comment #11
mandclu commentedComment #12
sclsweb commentedThe simple span change works for me. +1
The use case for the additional change to modal.css, using the [open] selector, is with the Bootstrap theme. I'm using drupal/bootstrap and the modal doesn't show at all when using modal with the text labels (not icons). It appears that Bootstrap loads with
.modal { display: none; }and expects a different trigger to show it, butdialog.add-to-calendar[open] { display: inline-block; }overrides it in a way that's compatible with how Add to Calendar works.I understand if you want to keep theme-specific fixes out of your code. The code could instead go in a Bootstrap subtheme CSS file and still work (or one could override addtocal-links--modal.html.twig to add the missing Bootstrap code), but-- maybe Bootstrap is still common enough that it might be worth it to have it just work out of the box?
Comment #13
tirupati_singh commented@mandclu, the css porperty
.add-to-calendar { display: inline-flex;}was being used for bothModal dialogandList of linksformatter while using the module and due to this css the modal was always being opened. On using the css porperty.add-to-calendar[open], span.add-to-calendar { display: inline-flex; }the modal issue resolved successfully, additionally using theopenattribute the css the icons in the modal also wrap as group for the narrow spaces.To reproduce the issue for
[open]: You just have to selectModal dialogin theLinks displayformatter and visit the node where the date field has been used. You'll notice that the modal is being open always and the icons on the modal for narrow spaces didn't wrap properly. This issue can be resolve for modal if we also use cssdisplay:inline-flex;only when the modal opens.And the changes in the MR !25 is working fine for the List of links for date field.
Comment #14
mandclu commentedWith the simpler change in MR !25 I can't reproduce any problem with modal display when using the Olivero theme. The assertion by @sclsweb in #12 that any remaining issues are specific to a different theme (like Bootstrap) seem plausible. I'm not against adding a small amount of code to make sure this module behaves well in Bootstrap and any subthemes, however.
I remain skeptical that it is necessary to add selector to the
display: inline-flex;declaration, however. From what I can see both it and the otherdisplay: inline-block;selectors will only target dialogs, and because the latter has higher specificity it will always apply. Can someone test MR !24 with a Bootstrap subtheme, and without the extra.add-to-calendar[open]selector in icons.css?Comment #16
mandclu commentedWithout more feedback on the Bootstrap-specific issue, I'm going to go ahead and commit a fix for the widespread issue. Please open a child issue for the Bootstrap problem, and provide additional feedback on the precise markup needed.