[Modal] Allow dynamic configs by reevaluating its properties on show

[TomONeill]

Prerequisites

• [X] I have searched for duplicate or closed feature requests
• [X] I have read the contributing guidelines

Proposal

My proposal is to reevaluate the passed options to the Modal constructor when show() is called.

A perfect working example of this is how Tooltip handles certain properties, like placement or title.

{
  "placement": () => this.placement,
  "title": () => this.title
}

I'd like to see similar behaviour for the modal, like so:

{
  "backdrop": () => this.shouldShowBackdrop,
  "focus": () => this.shouldFocus,
  "keyboard": () => this.allowKeyboard,
}

Motivation and context

We're creating a modal using its constructor early in the runtime of the application. Then at later points in time, we pass dynamic content to the modal and then call show. The only problem with this approach is that we can't change any of the configuration initially passed into the constructor, like keyboard, once the modal instance was created.

[DenisLopatin]

Hi. What about the following idea to implement? We can use a proxy object to track the configuration object. This will allow us to allow users to pass their custom functions to change the behavior of the component at runtime. Without modifying all other code in the library (hope).

js/src/modal.js

...
const DefaultType = {
  backdrop: '(boolean|string)',
  focus: 'boolean',
  keyboard: '(boolean|function)' // so far, only for keyboard
}

/**
 * Class definition
 */

class Modal extends BaseComponent {
  constructor(element, config) {
    super(element, config)

    this._dialog = SelectorEngine.findOne(SELECTOR_DIALOG, this._element)
    this._backdrop = this._initializeBackDrop()
    this._focustrap = this._initializeFocusTrap()
    this._isShown = false
    this._isTransitioning = false
    this._scrollBar = new ScrollBarHelper()

    this._addEventListeners()

    this._config = new Proxy(this._config, {
      get(target, prop, receiver) {
        const properties = Object.keys(Default)

        if (typeof prop === 'string' && properties.includes(prop)) {
          return typeof target[prop] === 'function' ? target[prop]() : target[prop]
        }

        return Reflect.get(target, prop, receiver)
      }
    })
  }
...

Usage example:

import { Modal } from '../js/index.esm.js'

const users = {}

const config = {
  focus: true,
  keyboard: () => Object.keys(users).length > 0,
  backdrop: 'static'
}

const fetchUsers = document.getElementById('keydown');
const button = document.getElementById('button');
const modal = new Modal('.modal', config)

button.addEventListener('click', () => {
  modal.show()
})

fetchUsers.addEventListener('click', async () => {
  const response = await fetch('https://jsonplaceholder.typicode.com/users')
  Object.assign(users, await response.json())
  fetchUsers.remove()
}, {once: true})

https://github.com/user-attachments/assets/46438318-72c1-4b68-a219-f1051e4a512d

If the proxy option is not being considered, could you give us some feedback? I am interested in this problem, I have come across it personally. I did not run the tests. It is not known how this will behave during testing.

[TomONeill]

Honestly, I'm not really sure about the internals of Bootstrap. However the implementation you shared and the video, that does look like what I'd be expecting.

Looking a little bit into the tooltip code, it doesn't look like a Proxy object is used there, but rather an execute() function: https://github.com/twbs/bootstrap/blob/main/js/src/tooltip.js#L374 coming from util: https://github.com/twbs/bootstrap/blob/main/js/src/util/index.js#L225

Couldn't that be implemented? That sounds a lot easier, also in regard to testing.

[DenisLopatin]

Honestly, I'm not really sure about the internals of Bootstrap. However the implementation you shared and the video, that does look like what I'd be expecting.

Looking a little bit into the tooltip code, it doesn't look like a Proxy object is used there, but rather an execute() function: https://github.com/twbs/bootstrap/blob/main/js/src/tooltip.js#L374 coming from util: https://github.com/twbs/bootstrap/blob/main/js/src/util/index.js#L225

Couldn't that be implemented? That sounds a lot easier, also in regard to testing.

Maybe it's actually much simpler. I'll try to figure it out. Thx

[TomONeill]

Sounds good mate! Thanks!

[DenisLopatin]

@TomONeill Yes, you were right. The execute function is already provided in the bootstrap api to perform the specified operation. That is, the next section of the code will do the same thing:

js/src/modal.js

     const keyboard = execute(this._config.keyboard, [this, [], this._element])

      if (keyboard) {
        this.hide()
        return
      }

[TomONeill]

Awesome, so technically this could become a PR, right?

[DenisLopatin]

Well, if the bootstrap team doesn't mind, that's it:

    if (this._config.keyboard) {
        this.hide()
        return
    }

will be displayed as (across the entire Modal api):

 if (execute(this._config.keyboard, [this, [], this._element])) {
        this.hide()
        return
 }

and if it passes the tests, then I think so)