-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add collapse all/expand all to Listcontrol #912
Conversation
Deploy preview ready! Built with commit af30171 |
Deploy preview ready! Built with commit af30171 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When all the items are collapsed/expanded manually, the "collapse/expand all" button should detect that and switch its functionality accordingly. e.g., if you collapse all the items manuallythe button should automatically switch to "expand all".
|
||
for (let i = 0; i < itemsCount; i++) { | ||
itemsCollapsed = itemsCollapsed.set(i, !allItemsCollapsed); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These last three lines can be replaced with something like
const newItemsCollapsed = Array(itemsCollapsed.length).fill(!allItemsCollapsed);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! The button updates it's state accordingly now.
But I sticked with the forloop, because I couldn't find a more slick solution. With the loop I can simultaneously create and set the list elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can remain as-is - @erquhart pointed out that replacing this is more involved than I thought and the for-loop is a pretty simple solution here.
@drlogout awesome work. I pushed a commit to update the design, good to merge. |
Regarding issue #928 list items are now collapsed by default. They can be expanded by adding - name: "authors"
label: "Authors"
file: "_data/authors.yml"
description: "Author descriptions"
fields:
- name: authors
label: Authors
widget: list
collapsed: true
expandItems: true
fields:
- {label: "Name", name: "name", widget: "string"}
- {label: "Description", name: "description", widget: "markdown"} What do you think? I needed to change |
@drlogout so EDIT: after re-reviewing, I'm ready to approve this PR once we've resolved the names of these options and the |
I didn't even notice the new field option - good idea @drlogout! So I discussed this with our designer friend @neutyp and he had a great point: collapsing the list itself isn't super useful to begin with - collapsing the children is really all we need. I agree with that, and I'm thinking we can go ahead and make "collapse/expand" here just refer to the children and remove the old collapse functionality. Regarding the field option name, I believe we should default these to collapsed when an entry is opened, so I'm thinking "expanded" is a good name for the option. |
Thanks for noting our conversation @erquhart — the original intent of the collapse action at the left of the 'function toolbar' in the list is meant to collapse all the children (the list items), not the parent (the whole list itself). I didn't have time to correct our incorrect implementation as we scrambled to hit the 1.0 launch date. We discussed and agreed on this functionality, but left open the idea of the icon changing in some way to better reflect the functionality. Do you still think an icon change is needed Shawn? |
@drlogout quick summary of my changes:
Thanks again for getting this in! |
Dismissing since Benaiah's concern was addressed.
- Add collapse all/expand all to Listcontrol
This adds a button to collapse and expand all list items without closing the whole list widget.
To sort list items with drag & drop it's more practical if they are collapsed. Without a collapse all/expand all switch you need to collapse every item manually.