IBX-3456 Collapse all button support #1394
Conversation
| const collapseAll = toggleAllBtn?.querySelector('.ibexa-attribute-group__toggler-collapse'); | ||
| const MULTI_COLLAPSE_BODY_CLASS = '.multi-collapse'; | ||
| let singleElementClicked = []; | ||
| const checkIfChangeText = () => { |
There was a problem hiding this comment.
I think the name of the function is wrong because this function only toggles the d-none class.
I have one more question, what will happen if it will be 2 or 3 collapse all buttons for different groups?
I would probably create a new JS file that contain all the collapse/expand logic for multiple collpase. The button in the data-attribute could be given a selector which collapse it should collapse/expand/listen to, etc.
There was a problem hiding this comment.
I proposed a new name for the function. In my opinion, based on the YAGNI principle, we should first discuss the probability of such a situation before making such changes.
| @@ -0,0 +1,37 @@ | |||
| .ibexa-multi-collapse { | |||
There was a problem hiding this comment.
Moved styles from product-catalog. Based on previous review comment let me know if I still should change class name to ibexa-pc-...
bozatko
force-pushed
the
IBX-3456-attribute-groups
branch
from
c1599eb to
c43983f
Compare
bozatko
force-pushed
the
IBX-3456-attribute-groups
branch
from
1228b09 to
c8db495
Compare
|
| section.addEventListener('click', () => { | ||
| const currentCollapsibleBtns = [...multiCollapseNode.querySelectorAll('[data-bs-toggle]')]; | ||
|
|
||
| window.clearTimeout(toggleAllTimeout); |
There was a problem hiding this comment.
here and other places: You have window under global
| window.clearTimeout(toggleAllTimeout); | |
| global.clearTimeout(toggleAllTimeout); |
| if (shouldBeToggled) { | ||
| toggleMultiCollapseBtn(currentToggleAllBtn, collapsedCount === 0); | ||
| } | ||
| }, 200); |
There was a problem hiding this comment.
create variable with this value that says what 200 is
| let toggleAllTimeout; | ||
| const toggleAllBtns = [...doc.querySelectorAll(`[data-multi-collapse-btn-id]`)]; | ||
| const toggleMultiCollapseBtn = (btn, changeToCollapseAll) => { | ||
| const displayedText = changeToCollapseAll |
There was a problem hiding this comment.
I will prefer create 2 variables and move them from this function next in toggleMultiCollapseBtn() just toggle them, but this is not must have :)
| const displayedText = changeToCollapseAll | |
| const collapseAllLabel = Translator.trans(/*@Desc("Collapse all)*/ 'product_type.edit.section.attribute_collapse_all', {}, 'ibexa_product_catalog'); | |
| const expandAllLabel = Translator.trans( /*@Desc("Expand all)*/ 'product_type.edit.section.attribute_expand_all', {}, 'ibexa_product_catalog'); |
Related PRs:
Description:
For QA:
Documentation: