backdrop-contrib / paragraphs

Paragraphs module to control your content flow
https://backdropcms.org/project/paragraphs
GNU General Public License v2.0
5 stars 11 forks source link

Enable modal administration - Sometimes modal does not open fully #140

Closed stpaultim closed 2 years ago

stpaultim commented 2 years ago

I understand that this is an experimental feature. The only problem that we've been having with it is that sometimes (not always) the modal does not open fully.

image

Usually, if we start scrolling on the page, the modal pops open and it's not a very big problem. But, in this specific test I did that work-a-round did not work. I had to close the modal and reopen it. Upon reopening, it opened correctly.

argiepiano commented 2 years ago

I experienced this same behavior with modals in other contexts (not opening correctly). I suspect it's a bug. I solved it by loading the js library backdrop.dialog.ajax in the page that contains the link that opens the dialog. As in adding backdrop_add_library('system', 'backdrop.dialog.ajax'); in a hook_preprocess_node() hook.

The problem was that the needed library that controls the size of the modal was not being loaded correctly - you had to load it explicitly

TeamTriplo commented 2 years ago

I just tested adding backdrop_add_library('system', 'backdrop.dialog.ajax'); in a hook_preprocess_node() hook and its working now. Now if anyone want to use in specific node they could easily get the node where want to put and make it working for specific node.

argiepiano commented 2 years ago

Ideally, as a potential fix for all situations, this library should be added within paragraphs_field_formatter_view() when the modal admin is enabled.

laryn commented 2 years ago

@argiepiano @stpaultim Would it make sense to add it here with these others using #attached?

https://github.com/backdrop-contrib/paragraphs/blob/1.x-1.x/paragraphs.module#L1839-L1844

argiepiano commented 2 years ago

Replacing 'backdrop.ajax' with 'backdrop.dialog.ajax' should be enough in: https://github.com/backdrop-contrib/paragraphs/blob/e1d39254e38cfccca5d323dda60293d485858154/paragraphs.module#L1844

Adding 'backdrop.dialog.ajax' will automatically add backdrop.ajax

But I have not tested whether this is the better location for this, or paragraphs_field_formatter_view

laryn commented 2 years ago

@stpaultim Are you willing to test the site that was having the issue with the change above?

stpaultim commented 2 years ago

We did get this working by hacking the module with some code based upon the suggestion by @argiepiano. I will ask my colleagues to chime in here with feedback on what worked for them and to test whatever patch we think will fix this problem. @laryn

laryn commented 2 years ago

Thanks @stpaultim -- if it works well, I think the cleanest would be to simply replacing 'backdrop.ajax' with 'backdrop.dialog.ajax' in the line discussed above. If that doesn't work well, let's try plan B (paragraphs_field_formatter_view).

bdalomgir commented 2 years ago

@stpaultim I just replaced 'backdrop.ajax' with 'backdrop.dialog.ajax' in paragraphs.module at line 1844 and it's works for me. Thanks @argiepiano

stpaultim commented 2 years ago

@bdalomgir - Would you please create a PR for this module with that change? That will make it easier to discuss and test.

Atten: @laryn and @argiepiano

stpaultim commented 2 years ago

@bdalomgir - Thanks for PR - https://github.com/backdrop-contrib/paragraphs/pull/142

laryn commented 2 years ago

Merged. Thanks @bdalomgir, @stpaultim, and @argiepiano!