-
-
Notifications
You must be signed in to change notification settings - Fork 31.3k
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
Fixed #27471 -- Made admin's filter choices collapsable. #15424
Fixed #27471 -- Made admin's filter choices collapsable. #15424
Conversation
10ff20a
to
086d161
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.
Thanks for this @mgaligniana.
I think I much prefer the <details>
approach over rebuilding that by hand, so let's focus effort here for the now. (We can always revert to the original approach if it doesn't work out, but it should...)
Two initial queries:
- Can we style the disclosure element at all? The ▼/▶. Maybe that's fine, but are there options?
- Can you think about @kezabelle's point from Fixed #27471 -- Make admin's list_filter choices collapsable #15331 about persistence? Otherwise I collapse the irrelevant filters, click a remaining relevant one, which triggers a reload, and then all the filters are back how they started. Session Storage should be fine. I guess a (sparse) per-changelist, per-filter reference to control the
open
state, applied on load, and stored on a change. … 🤔
I don't think I'd worry about auto-collapsing (< 10
) here.
Here's a snippet which works well to remove the default styles: summary {
list-style: none;
&::-webkit-details-marker {
display: none;
}
} Now you can add custom styles via |
Thanks @matthiask and @carltongibson
Thank you again! |
2d829d7
to
a8e1b43
Compare
dfe301e
to
b13259d
Compare
b13259d
to
a64d6e4
Compare
@@ -11,6 +11,7 @@ | |||
<link rel="stylesheet" href="{% static "admin/css/nav_sidebar.css" %}"> | |||
<script src="{% static 'admin/js/nav_sidebar.js' %}" defer></script> | |||
{% endif %} | |||
<script src="{% static 'admin/js/filters.js' %}" defer></script> |
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.
Not sure if this is the correct to place the file and if the name is accurate 🤔
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 moved it to the templates/admin/change_list.html
.
a64d6e4
to
19d841d
Compare
} | ||
|
||
#changelist-filter details > summary::before { | ||
content: '▶️'; |
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.
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.
Can we use arrows instead of emojis? ↓ → 🤔
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'm not an UX expert, but they look nice and the same in all browsers:
Maybe bolded and with --link-hover-color
to make it clear that they are clickable:
#changelist-filter details > summary::before {
content: '→';
font-weight: bold;
color: var(--link-hover-color);
}
#changelist-filter details[open] > summary::before {
content: '↓';
font-weight: bold;
color: var(--link-hover-color);
}
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 really like it @felixxm! Styles updated.
77160ca
to
fd8f894
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.
@mgaligniana Thanks 👍 I pushed small edits and updated screenshots in docs.
@@ -11,6 +11,7 @@ | |||
<link rel="stylesheet" href="{% static "admin/css/nav_sidebar.css" %}"> | |||
<script src="{% static 'admin/js/nav_sidebar.js' %}" defer></script> | |||
{% endif %} | |||
<script src="{% static 'admin/js/filters.js' %}" defer></script> |
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 moved it to the templates/admin/change_list.html
.
@felixxm I know this is not the appropriate place to ask a question unrelated to the pull request but I just read the Django contributing Git guideline and it says:
I noticed mine is my GitHub username. Does it affect in any way? Should we document the reason? |
fd8f894
to
b42488b
Compare
Don't worry it's not a blocker. I updated the author's description. |
b42488b
to
27aa703
Compare
This is the 2nd approach to solve ticket 27471 following the knyghty and pauloxnet suggest using details/summary elements
You can run test doing:
./runtests.py admin_changelist.tests.SeleniumTests.test_collapse_filters --selenium=chrome,firefox