I place the below link in a body field of a node.
<a class="a-action purchase-inquiry use-ajax" data-dialog-options="{&quot;dialogClass&quot;:&quot;pidia&quot;,&quot;width&quot;:&quot;900&quot;}" data-dialog-type="modal" href="/">Purchase Inquiry</a>

On bartik this link opens a modal with a width of "900" and a class called "pidia". If I switch to bootstrap theme neither of these values gets passed. Can this be applied to bootstrap modals also.

Comments

Jalite1991 created an issue. See original summary.

Jalite1991’s picture

Status: Active » Closed (works as designed)

Fix my own problem. Bootstrap theme has a setting to turn off bootstrap modals in the Appearance Setting page.

Jalite1991’s picture

Category: Bug report » Feature request
Issue summary: View changes
Status: Closed (works as designed) » Needs work

Actually I am changing this to a feature request. As of right now the Bootstrap Modal Js is the better option (As far as styles and fading effects). Can we allow developers to pass additional classes that need to be passed to the modal via the data-dialog-options attribute. It seems likes bootstraps Modal.js is setting this to a empty string instead applying the additional settings passed.

Jalite1991’s picture

Issue summary: View changes
markhalliwell’s picture

Status: Needs work » Closed (won't fix)

Because Bootratrap doesn't have options like Drupal does.

It handles options only on initialization of the modal via a JS object or specified data attributes: http://getbootstrap.com/javascript/#modal

It does not have any option for "width" or additional "class".

You will have to add this functionally manually yourself if you want it.

Jalite1991’s picture

Thanks for the quick response. So question, since the bootstrap theme code is overriding the drupal dialog.js way of activating a modal wouldn't this be a bug in the code?

markhalliwell’s picture

No. They're two entirely different types of modals/JS.

Jalite1991’s picture

I agree that they are different js scripts, but if the code I mentioned in my original post acts one way on a fresh install of core, and a totally different way after installing and activating bootstrap. Doesn't that mean bootstrap is overriding the behavior of how core is calling the modals. Only after turning on the bootstrap theme does the ability to pass additional classes/options cease to happen.

I won't pretend to be an expert about how the Drupal Bootstrap theme is working, but if I'm not mistaken the bootstrap framework doesn't activate modals using the link structure I mentioned. So this means something additional was added to make this happen. Am I missing something about the theme is accomplishing this?

markhalliwell’s picture

Category: Feature request » Support request
Priority: Major » Minor

acts one way on a fresh install of core, and a totally different way after installing and activating bootstrap

That is because the Bootstrap Framework has it's own Modal JS API and this base theme simply integrates with it the best it can. What did you expect when you installed a base theme that integrates an existing external framework that has it's own ways of doing things?

Doesn't that mean bootstrap is overriding the behavior of how core is calling the modals.

Yes, but only within the simplifications/limitations of the Bootstrap APIs.

if I'm not mistaken the bootstrap framework doesn't activate modals using the link structure I mentioned

The Bootstrap Framework itself does not, no. However, that is why this base theme exists. It helps override core's "dialog" JS API so it utilizes the framework's modal APIs instead:

http://cgit.drupalcode.org/bootstrap/tree/js/misc/dialog.js and
http://cgit.drupalcode.org/bootstrap/tree/js/misc/dialog.ajax.js

This is necessary because the base theme does not rely on jQuery UI (which is what core utilizes for it's dialogs). Nor is there any need to because Bootstrap provides it's own Modal APIs. This prevents an entirely unnecessary library from being loaded and modals not styled like the rest of the theme.

Furthermore, setting a width on a Bootstrap modal is also pretty much a non-starter. Bootstrap has specific styling around modals and breakpoints that match the rest of the theme. Any customization of setting a specific "width", would have to be implemented in a completely custom manner.

Simply put: you cannot expect all of Drupal's APIs to work with an existing external framework like Bootstrap. That doesn't mean it's a "bug". Sometimes it's simply because that aspect hasn't been fully flushed out (due to various reasons/availability). In this case, however, it's because it just doesn't make sense to do so.

As I said above, if you absolutely need this type of "functionality", you will have to implement a custom solution (likely your own sub-theme JS overrides/additions). This base theme is meant to get you started along the right tracks. It isn't meant to solve every single use-case.

Jalite1991’s picture

I believe I see what your saying now. Just to answer your questions.

What did you expect when you installed a base theme that integrates an existing external framework that has it's own ways of doing things?

I would expect core (or base theme) features to continue operating the way it did before the theme was added. I don't believe it's a stretch to think that a theme shouldn't remove functionality but extend it. Especially in the case of bootstrap, since it has it's own method of achieving the same thing. I would expect the theme to append whatever it felt was necessary after the Drupal method was completed in order to achieve the consistent styling. If that wasn't possible then users would have to rely on Bootstrap method.

Setting a width on a Bootstrap modal is also pretty much a non-starter.

I agree, but you have completely ignored that fact that you can set classes as well using the same method. Which would completely useful to anyone who needed to style modals in different ways depending on the situation (form vs dialog vs content preview for example). Unless the bootstrap theme has implemented a way for developers to pass classes to help style the modal. This method of "simplifying/limiting" what drupal is able to seems counter productive.

You are basically saying in order to make something look nice you broke pieces of it off, and if we want it to work again we have put it back together again.

This base theme is meant to get you started along the right tracks. It isn't meant to solve every single use-case.

Just to be clear, I'm not trying to be argumentative. Nor do I believe that this should solve every problem out of the box. Quite the opposite actually. I love the flexibility this theme provides. It is one of the most downloaded themes for obvious reasons. I'm simply stating that in a effort to integrate Bootstraps version of modals into what Drupal was doing it seems like something really useful was sacrificed, and I was hoping to get back.

Still, I will not push it any further. Thank you for your time, and I hope the thread will provide someone with closure if they have the same concern I did.

markhalliwell’s picture

I would expect core (or base theme) features to continue operating the way it did before the theme was added.

Then you don't understand how [external] themes work.

I don't believe it's a stretch to think that a theme shouldn't remove functionality but extend it.

A 1:1 parity of features isn't always going to happen, especially when said features simply don't exist in an external framework like Bootstrap.

Especially in the case of bootstrap, since it has it's own method of achieving the same thing.

Bootstrap does not have the same methods to achieve the same thing.

Which would completely useful to anyone who needed to style modals in different ways depending on the situation (form vs dialog vs content preview for example)

I ignored it because it's certainly possible to add classes (or do anything else you want) via Bootstrap's modal events.

Unless the bootstrap theme has implemented a way for developers to pass classes to help style the modal. This method of "simplifying/limiting" what drupal is able to seems counter productive.

It's not. As mentioned above, the proper way to do any customization of modals is to use the Bootstrap API.

You are basically saying in order to make something look nice you broke pieces of it off, and if we want it to work again we have put it back together again.

I'm not saying that at all, you are. We didn't "break" anything, we simply didn't add custom JS in the base theme for "features" that are rarely used and often in an inappropriate way.

Styling modals in this fashion is an anti-pattern and definitely not a "Best Practice™" in Drupal theming. If you need to customize these types of theming components, you should customize them in the theme itself (via CSS and JS), not via a module's PHP implementation.

The "proper" way to do this (and I have done it) is to set some sort of "context" on the module side. This can be either using the #context property on the render array element itself or via a data attribute. Then you detect this context in a sub-theme somehow (via a preprocess, prerender, etc.) to add any additional context that will be used in modal initialization.

Finally, you utilize aforementioned Bootstrap modal APIs to add the classes or set anything else you want on the modal itself before actually initializing it.

Just to be clear, I'm not trying to be argumentative.

Perhaps not, but it does seem like you're not understanding some pretty fundamentals of how JS libraries work.

I'm simply stating that in a effort to integrate Bootstraps version of modals into what Drupal was doing it seems like something really useful was sacrificed, and I was hoping to get back.

Nothing was "sacrificed", it simply was never implemented because the framework itself has nothing to actually implement said "features", natively.

Perhaps this will help explain all this a bit better:

Core utilizes jQuery UI which has a public options method that can be invoked (e.g. $.dialog('options', name, value)). This is what allows data-dialog-options to work because that JSON encoded value is passed directly to jQuery UI.

Bootstrap does not utilize jQuery UI at all and thus, has no public "option" method that can be used. The only way you could possibly pass these options is upon initialization of the modal, which, coincidentally doesn't work with Bootstrap's data APIs.

Furthermore, you're also expecting these data-dialog-options property names to be a 1:1 map between jQuery UI and Bootstrap.

It doesn't work that way.

These are two entirely different jQuery libraries that name things completely different.

The "options" that jQuery UI uses for it's "dialog" plugin simply do not work on the "options" that Bootstrap establishes.

This is why relying on "dialog options" from core's "API" is probably the worst thing you could actually do.

Just because core implements something, doesn't mean it's perfect. There are a lot of "theming APIs" that get thrown into core by people who don't actually understand theming to begin with. They often make assumptions about themes and what libraries will be used, this is a perfect example of one.

So, again, if you need to customize your modals, do it on the JS level in your sub-theme.

akalata’s picture

It's not very helpful when a theme silently disregards an established API (regardless of whether or not Core's implementation decisions are good ones). I know this is an old issue, and that project documentation is always evolving, but if we still need a better way to say "this theme does not utilize jQuery UI, so some core JS features will not work the same way" let's figure that out.

Increasing this confusion is the fact that `data-dialog-options` is always passed through, even when it isn't used. I know that asking a theme to preprocess every single link to remove potentially-unused text is a ridiculous time sink -- but so is trying to debug options that you don't realize aren't being used.

Even MORE confusing is that some dialog options from jQuery UI ARE supported, `dialogClass` being one that I have encountered.

markhalliwell’s picture