search

backdrop-contrib / references_dialog

Extends reference fields by adding links to add, edit, and search for references through a dialog.
https://backdropcms.org/project/references
GNU General Public License v2.0
1 stars 3 forks source link

Dialogs layout broken since 1.3.4 #4

Closed ghost closed 1 year ago

ghost commented 8 years ago

Dialogs as shown with full default layout, due to change in layout selection in 1.3.4 see #1491

herbdool commented 6 years ago

The original commit fixed the issue even though not ideal -- notes in code on a better fix.

olafgrabienski commented 3 years ago

I'm not sure if this is the right issue and/or if I should open a new one: In contrast to the D7 module, in the Backdrop version there are unnecessary and therefore disturbing elements in the References Dialog's node form. See the highlighted area in the screenshot below. Is it possible to suppress the .l-header element within the iframe somehow? I'm also interested in a workaround.

screen-ref-dialog-edit

olafgrabienski commented 3 years ago

The original commit fixed the issue even though not ideal -- notes in code on a better fix.

@herbdool In my understanding the issue is not fixed but I'm not sure what you meant by "fixed ... even though not ideal". Is the screenshot above expected behavior?

olafgrabienski commented 2 years ago

Guess I've found the related core issue: https://github.com/backdrop/backdrop-issues/issues/1728

It has even a PR, updated by @klonos in Sep 2019!

herbdool commented 2 years ago

@olafgrabienski I don't recall anything about this anymore. Not whether it's fixed nor what is supposed to look like.

indigoxela commented 2 years ago

There already is a way to suppress the surrounding layout with class="use-ajax" data-dialog="true" as attributes for a link. But it has bugs. See https://github.com/backdrop/backdrop-issues/issues/5250

olafgrabienski commented 2 years ago

Thanks for your feedback, @herbdool and @indigoxela ! In the meantime, I've tested the PR for the related core issue. It fixes the issue for me in combination with uncommenting one line in references_dialog.module.

So, as soon as the core issue is fixed, let's make the needed change in References Dialog.

herbdool commented 2 years ago

I think the better approach would be to adapt this module to use what @indigoxela mentions: class="use-ajax" data-dialog="true". I did a quick test to confirm it should work, but needs some work to ensure the behaviour matches.

olafgrabienski commented 2 years ago

@herbdool I'm fine with every approach, couldn't tell their advantages and disadvantages. Why do you think class="use-ajax" data-dialog="true" is better?

indigoxela commented 2 years ago

Maybe @robertgarrigos wants to share his thoughts regarding iframe vs. use-ajax?

olafgrabienski commented 2 years ago

Also interesting:

[References Dialog] contains interesting sample code on how to use jquery dialog for an iframed content (which solves complications and issues encountered when using plain forms with Dialog API)

https://github.com/backdrop-ops/contrib/issues/138#issue-135093161

indigoxela commented 2 years ago

A possible solution (small change) has been posted in this comment: https://github.com/backdrop/backdrop-issues/issues/1728#issuecomment-926423499

robertgarrigos commented 2 years ago

Sorry @indigoxela I don't have either a clear position on which option would be best.

herbdool commented 2 years ago

I think we're just waiting for a core PR to be merged then this module can call the new function to suppress the layout.

indigoxela commented 2 years ago

I think we're just waiting for a core PR to be merged

It got merged, but only in 1.21.0, which means, it will be part of the minor release in January.

robertgarrigos commented 1 year ago

A possible solution (small change) has been posted in this comment: backdrop/backdrop-issues#1728 (comment)

Does this apply any more? I applied that change locally and couldn't see any effect.