-
Notifications
You must be signed in to change notification settings - Fork 108
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 child, sibling, and archive controls to filterable lists #6039
Conversation
c97a4dc
to
59d5e46
Compare
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 works perfectly -- thank you, @willbarton!
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.
I don't totally understand the alchemy behind why the Activity Log works, but it does. Not approving quite yet because I have a couple significant questions.
filter_children = blocks.BooleanBlock( | ||
default=True, | ||
required=False, | ||
help_text=( | ||
"If checked this list will only filter its child pages." | ||
), | ||
) | ||
filter_siblings = blocks.BooleanBlock( | ||
default=False, | ||
required=False, | ||
help_text=( | ||
"If checked this list will only filter its sibling pages." | ||
), | ||
) |
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 both being booleans with the help text as-is opens the possibility for confusion about what happens if both are checked. The logic in the model checks filter_children
and then only if that is False
does it check filter_siblings
, but that's not evident in the UI created by these block definitions.
- Should we allow for the possibility of both being true, i.e. a list showing both siblings and children? Grandchildren or further descendants? Nieces and nephews?
- If not, could we make this a ChoiceBlock defaulting to children?
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.
You're right in that the behavior of the booleans is non-obvious in the way they interact. I don't think there's much to do to help that right now – a ChoiceBlock
might work, but I think it'd add some additional complexity in the implementation that might not be worth it (more on that in a moment). As a general thing, the way archival appears in the admin is already going to be confusing and non-obvious until we actually spend time designing that experience.
For filterable lists themselves, I think a better way than any of this would be to expose the parent page to filter as a page selector, or, if no page is selected, allow category selection in the admin. But that has a lot more implications that we'd need to handle, and potentially more pages that could break.
I will also note, that the reason the filter boolean fields are on the filterable list block itself, rather than the page models, is the result of consulting with our UXers. @niqjohnson rightly points out that the filterable list block is where we already include some configuration for how the list of filtered pages renders, and so it makes sense to include these fields in that context. |
This change moves the "filter_children" property off the `FilterableListMixin` and places it as a boolean control in the `FilterableListBlock` organism. It also adds corresponding `filter_siblings` and `filter_archive` boolean controls. This will enable the content editor in the CMS to configure filterable list page behavior to filter either children or siblings, and/or whether to filter archived content or not. This includes a migration to add the controls and a data migrations to ensure that the existing `filter_children` behavior of `NewsroomLandingPage` and `ActivityLogPage` is maintained.
This should help make it clear that children will take precedence over siblings.
d1281f4
to
812b739
Compare
This change moves the "filter_children" property off the
FilterableListMixin
and places it as a boolean control in theFilterableListBlock
organism. It also adds correspondingfilter_siblings
andfilter_archive
boolean controls.This will enable the content editor in the CMS to configure filterable list page behavior to filter either children or siblings, and/or whether to filter archived content or not.
This will allow for archive filterable list pages to live inside parent filterable list pages and filter sibling pages. For example, the "Blog" filterable list page, set to filter child pages, can now have a corresponding "Blog Archive" beneath it, set to filter siblings and archived pages, and one can create any number of content combinations across either page to link back and forth between them.
This PR also includes a migration to add the controls and a data migrations to ensure that the existing
filter_children
behavior ofNewsroomLandingPage
andActivityLogPage
is maintained.The field labels and help text in the admin can be iterated on as needed, but this PR will get the desired functionality in for the 9/30 deadline.
Screenshots
Checklist