(Enhancement) And what if there are 2 close buttons in the Alert component, when you click on any of these buttons, the Alert should close

[cvaize]

And what if there are 2 close buttons in the Alert component, when you click on any of these buttons, the Alert should close. This test is performed 2 times, once on each button. When you click on the 2nd button, nothing happens. var el = document.querySelectorAll('[data-dismiss="alert"]')[0]; var newEl = el.cloneNode(true); newEl.style.right = '30px'; el.parentElement.insertAdjacentElement('afterbegin', newEl); new BSN.components.Alert(el); new BSN.components.Alert(newEl);

[thednp]

This probably works with the original plugins, simply because the event delegation is managed at the document level. Other than that, I don't see an useful enhancement for our component.

[cvaize]

I can not understand the reason for using only the first button, because if this button is in Alert, and it was clicked, it must close the current Alert, otherwise we have limited the functionality of one button in advance.

[cvaize]

[thednp]

You can do something even more simple:

<a href="#" onclick="this.closest(.alert).querySelector('[data-dismiss="alert"]').Alert.close()">
 Your content link
</a>

It doesn't look very bad to me, and it should work without any modifications.

EDIT:

<button class="border-0 p-0 bg-transparent text-primary" onclick="this.closest('.alert').querySelector('[data-dismiss=\'alert\']').Alert.close()">
Your content link close
</button>

[cvaize]

You can, but why limit it to this code (element === e.target || element.contains(e.target)). My reasoning is this: if you click on the close Alert button, and above this button is an Alert, the listener exactly worked on the button on the Close button, since it could not work not on it, it means that the Alert needs to be closed. The code sounds like this, if there is an Alert above the element that was clicked, and inside there is any first close button Alert, then check that any found, the first close button is the button that was clicked. I think this is unnecessary and I don't see the point in this check.

[thednp]

Look I even made a small teak:

<button data-title="Close this alert via &lt;code&gt;close()&lt;/code&gt; method" data-tip="tooltip" class="border-0 p-0 bg-transparent text-primary" onclick="dismissAlert.call(this)">
 Your content Link
</button>

function dismissAlert(){
  this.Tooltip && this.Tooltip.dispose();
  this.closest('.alert').querySelector('[data-dismiss="alert"]').Alert.close()
}

How do you like that?

[thednp]

You can, but why limit it to this code (element === e.target || element.contains(e.target)). My reasoning is this: if you click on the close Alert button, and above this button is an Alert, the listener exactly worked on the button on the Close button, since it could not work not on it, it means that the Alert needs to be closed. The code sounds like this, if there is an Alert above the element that was clicked, and inside there is any first close button Alert, then check that any found, the first close button is the button that was clicked. I think this is unnecessary and I don't see the point in this check.

The first reason is this: we have on(element,'click',clickHandler) not on(alert,'click',clickHandler), we need to make sure user ONLY close alert via the designated dismiss button and AVOID any crashes we used to have back in version 1, that was 5 years ago btw.

The second reason for the extensive check is this: many people want to use custom graphics, even SVG, say

<button class="close" data-dismiss="alert"><svg><path d=".."></path></svg></button>

Now how do you expect this to work with your suggested change?

[thednp]

@cvaize what is your take on this please? Over the years we had to come to a way to handle all components clickHandler so they can accommodate situations like the above, I don't see a point in changing them.

I really think we should shift focus into #332, complete tests for all components so we can move on to documentation.

[cvaize]

@thednp I'll look into it tomorrow