IBX-3619: Tag update helper #633
Conversation
GrabowskiM
force-pushed
the
IBX-3619-update-tag-functionality
branch
from
66d790e
to
9065655
Compare
| }); | ||
| }); | ||
| const parseAll = (baseElement = doc) => { | ||
| if (!baseElement) { |
There was a problem hiding this comment.
baseElement has default, so when it can be undefined?
There was a problem hiding this comment.
It won't be undefined but it can be null.
|
|
||
| baseElement.dataset.bsOriginalTitle = content; | ||
| contentElements.forEach((contentElement) => { | ||
| contentElement.innerHTML = content; |
There was a problem hiding this comment.
this probably should be escaped
There was a problem hiding this comment.
so, e.g. innerText = content or maybe something more sophisticated?
There was a problem hiding this comment.
we have text helper for escaping.
| {% set tag_attributes = tag_attributes|default({})|merge({'class': (tag_attributes.class|default('') ~ ' ibexa-tag')|trim}) %} | ||
| {% set is_deletable = is_deletable is defined ? is_deletable : true %} | ||
| {% set tag_attributes = tag_attributes|default({})|merge({ | ||
| 'class': (tag_attributes.class|default('') ~ ' ibexa-tag' ~ (is_deletable ? ' ibexa-tag--deletable'))|trim |
There was a problem hiding this comment.
| 'class': (tag_attributes.class|default('') ~ ' ibexa-tag' ~ (is_deletable ? ' ibexa-tag--deletable'))|trim | |
| class: (tag_attributes.class|default('') ~ ' ibexa-tag' ~ (is_deletable ? ' ibexa-tag--deletable'))|trim |
| @@ -24,6 +24,7 @@ | |||
| } | |||
|
|
|||
| &__remove-btn { | |||
| display: none; | |||
There was a problem hiding this comment.
Wouldn't such a change make this an ibexa-badge? 🤔
There was a problem hiding this comment.
Well, yes, but actually no.
It looks the same way as badge when is_deletable set to false, but when it can be deleted it's not. And I don't think it would be wise to change whole rendered/used component based on only is_deletable param.
| if (!baseElement) { | ||
| return; | ||
| } | ||
|
|
||
| const middleEllipsisContainers = [...doc.querySelectorAll('.ibexa-middle-ellipsis')]; |
There was a problem hiding this comment.
Wouldn't this find also baseElement?
| if (!baseElement) { | ||
| console.warn('No baseElement provided'); |
There was a problem hiding this comment.
Printing this warning for null but not for undefined may be confusing. Maybe we can create more/less specific message? Or maybe create a more specific check?
| }); | ||
| parseAll(baseElement); |
There was a problem hiding this comment.
you should not use deprecated methods.
| }; | ||
|
|
||
| ibexa.addConfig('helpers.ellipsis.middle', { | ||
| parseAll, | ||
| update, |
There was a problem hiding this comment.
parseAll is deprecated and should not be used, we don't want to expose parse method?
GrabowskiM
force-pushed
the
IBX-3619-update-tag-functionality
branch
from
9443ed8
to
6430d85
Compare
GrabowskiM
force-pushed
the
IBX-3619-update-tag-functionality
branch
from
6430d85
to
485a241
Compare
GrabowskiM
force-pushed
the
IBX-3619-update-tag-functionality
branch
from
485a241
to
2d9311a
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Checklist:
$ composer fix-cs)