Modal component: bs-modal css variables not defined

[jmestxr]

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

No response

Tell us about your environment

MacOS Ventura

MarkBind version

5.0.1

Describe the bug and the steps to reproduce it

Bug description:

To reproduce the bug, set the bootswatch theme to any theme other than default and use a modal component.

When using a (non-default) bootswatch theme, bs-modal css variables for modal component are not found, resulting in it not being styled properly:

bootswatch theme used: 'sketchy'

Upon further probing, I found out this was due to bs-modal css variables not being registered:

These variables are defined in bootstrap's modal class, which were not specified in the Modal vue component:

modal class not specified in Modal component

Fix:

• Add modal class in vue component
• bootstrap's modal class contains a display: none. To override this, I added a display: block to modal-dialog class to fix this so the modal can be seen.

Additional notes:

• This does not happen in 'default' bootstrap theme, as modal css variables are not used in the bootstrap.min.css file for default theme

Expected behavior

No response

Anything else?

No response

[tlylt]

Hey @jmestxr, thank you for opening this issue! Great observation and I can indeed reproduce this.

Before we proceed with the fix suggested in #2354, may I know if you have found the reason why it works in default but not in other themes?

• Yes there are missing CSS variables like --bs-modal-bg, but why are they missing?

I took a brief look and observe that bootswatch themes uses "Bootstrap v5.2.3" while markbind by default ships "Bootstrap v5.1.3". Is that the underlying reason?

• If so, could the solution be that we need to pin the bootswatch version that we ship with so that they are in sync with our Bootstrap version? Also, does it mean that when we migrate our Bootstrap version to v5.2, the default theme will have the same problem?
• If that's the case, is there a better solution to deal with this modal CSS issue once and for all?

Please feel free to investigate further and let me know if you have any insights on this!

[jmestxr]

• Yes there are missing CSS variables like --bs-modal-bg, but why are they missing?

It seems they are missing because those variables are defined locally within the .modal class of bootstrap.min.css.
Hence for the fix, I added the modal class to vue-final-modal which wasn't added previously:

I took a brief look and observe that bootswatch themes uses "Bootstrap v5.2.3" while markbind by default ships "Bootstrap v5.1.3". Is that the underlying reason?

Yeah. It seems ur right! For default theme, we are using bootstrap.min.css that is using v5.1.3, which does not use any --bs-modal variables:

Whereas for the bootstrap.min.css that is using v5.2.3, they are being used:

The other bootswatch themes are using Bootstrap v5.2.3 for their bootstrap.min.css, where --bs-modal variables are used.

• If so, could the solution be that we need to pin the bootswatch version that we ship with so that they are in sync with our Bootstrap version?

This wouldn't seem to be the solution because the problem lies that we didn't specify .modal class in our vue-final-modal, where --bs-modal variables are defined locally.

However to ensure that the same styling behavior for default and other themes, I agree we should sync bootswatch and bootstrap versions.

Also, does it mean that when we migrate our Bootstrap version to v5.2, the default theme will have the same problem?

Yeah, because bootstrap.min.css for default theme is using --bs-modal variables within .modal class, which is not specified currently.

• If that's the case, is there a better solution to deal with this modal CSS issue once and for all?

Yeah, perhaps we can instead downgrade all bootswatch versions to 5.1.3. which can be downloaded here, to be on sync with the current Bootstrap version.

But if we were to upgrade to Bootstrap 5.2.3 in future, we will need to specify modal class in vue-final-modal, which is what my fix is doing currently.

[tlylt]

This wouldn't seem to be the solution because the problem lies that we didn't specify .modal class in our vue-final-modal, where --bs-modal variables are defined locally.

Have you tried downgrading bootswatch to 5.1.3 (or any higher version that ships with the same bootstrap version?...I'm not sure how they version their package) and see if the modals work for all themes?

But if we were to upgrade to Bootstrap 5.2.3 in future, we will need to specify modal class in vue-final-modal, which is what my fix is doing currently.

In that case, it seems like we should assess whether migrating to Bootstrap 5.2.3 is feasible, which is a separate, larger issue to explore (feel free to tackle this if you are interested:)

Specifically for this issue though, were you able to find out from Bootstrap's end why did they make such a change for the modal CSS and any guidance from them on how to best migrate it?

[jmestxr]

Have you tried downgrading bootswatch to 5.1.3 (or any higher version that ships with the same bootstrap version?...I'm not sure how they version their package) and see if the modals work for all themes?

I tried downgraded to bootswatch 5.1.3 and the modals are working!

Specifically for this issue though, were you able to find out from Bootstrap's end why did they make such a change for the modal CSS and any guidance from them on how to best migrate it?

Seems like bootstrap made the change to include component variables (not just for modals) for easier theming: https://blog.getbootstrap.com/2022/07/19/bootstrap-5-2-0/

[tlylt]

I tried downgraded to bootswatch 5.1.3 and the modals are working!

Yes, that's great, we should downgrade first before making a further move. Bootstrap touches a lot of things in MarkBind and having an unpinned version is pretty problematic.

Seems like bootstrap made the change to include component variables (not just for modals) for easier theming: blog.getbootstrap.com/2022/07/19/bootstrap-5-2-0

Thank you. Pretty interesting. More details available here in the CSS variables page: https://getbootstrap.com/docs/5.2/customize/css-variables/#component-variables

---

In our case, the root cause seems to be due to how we use just the styling of bootstrap modal classes when we implemented the new modal functionality via the vue-final-modal package, i.e. we didn't rely on Bootstrap for hiding/showing the modals.

Hence, even though the actual usage from Bootstrap's docs require us to have the .modal class in the outermost layer, we didn't.

https://getbootstrap.com/docs/5.2/components/modal/#modal-components

When it got to Bootstrap v5.2, the lack of .modal resulted in the missing CSS variables.

Here's my suggestion for the solution:

• add in .modal class, but at the right layer, (i.e. outside of the layer containing class="modal-dialog")
  • the current layer in your PR may not be appropriate. We could use <vue-final-modal classes="modal" https://v2.vue-final-modal.org/guide/properties#classes to put it just one layer above modal-dialog
• add the display=block for the .modal class to sort of "disable the display toggling provided by bootstrap
  • add an inline comment to explain why we added this. In fact, we can replace block with initial/unset/inherit/revert as well, whichever is more suitable semantically
• pin the bootswatch npm dependency in packages/core to "bootswatch": "5.1.3" (instead of ^5.1.3)
  • note that the dependency installation step in our DG is not working, I will raise an issue for that
• update test as required
• optional: call out this coupling of bootstrap and bootswatch in our developer guide
• raise an issue to track migration of bootstrap version
• verify that existing modal functionalities are all working, for both default, and themed

@jmestxr feel free to tackle the above in your PR #2353!