UCPKN-3880: Pass attributes to modal and toast sections. #693
Conversation
|
🚀 Deployed on https://preview-693--oelibrary.netlify.app |
| {% if _with_body_wrapper %} | ||
| </div> | ||
| {% endif %} | ||
| {{- _toast.custom_content|default('') -}} |
There was a problem hiding this comment.
kp77
changed the title
| { | ||
| role: "alert", | ||
| aria_live: "assertive", | ||
| with_close: false, |
There was a problem hiding this comment.
can we add another test without close?
There was a problem hiding this comment.
you can use the same data, but overwritting in test like this:
render({ ...demoData, with_close: false }),
src/data/toasts/data.js
Outdated
| with_close: true, | ||
| header: "Toast title", | ||
| header_attributes: new drupalAttribute().addClass("test-class"), | ||
| button_attributes: new drupalAttribute().addClass("test-class"), |
There was a problem hiding this comment.
I've updated in another pr the drupalAttributes package and when that pr is going to be merged we will need to change to DrupalAttribute from drupalAttribute.. I will take care of it, no worries
| <div{{ _body_attributes.addClass(['toast-body']) }}> | ||
| {{- _body -}} | ||
| </div> | ||
| {% if _with_body_wrapper %} |
There was a problem hiding this comment.
I think we didnt approach correctly these toasts in the first version and now with your changes, it becomes even more complex.. I think we can simplify it but I need to think of the impact
I dont like: _body_wrapper_classes _with_body_wrapper parameters and by removing them we would have a more simple template. We can leave it like this for now.
There was a problem hiding this comment.
Yes, I was also wondering about the body wrapper (there's no mention of that in the Bootstrap docs).
| {% set button_attributes = create_attribute() | ||
| {% set _button_attributes = _button_attributes | ||
| .addClass(['btn-close']) | ||
| .setAttribute('aria-label', 'close') |
There was a problem hiding this comment.
I would change this as if _button_attributes has aria-label attribute it would be overwritten by this
There was a problem hiding this comment.
Fixed it, also made the label capitalized 'Close', because that's the convention in other components (alert, modal).
|
Thanks @kp77 for this, 2 small points to update:
|
2 participants
No description provided.