#11868 closed New feature (fixed)
Multiple sort in admin changelist
Reported by: | Ben Davis | Owned by: | Ben Davis |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | admin sort multisort ordering order |
Cc: | andy@…, Patrick Kranzlmueller, idan@…, Andy Baker | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've made an attempt at adding multiple sort columns to the admin changelist. I'm basically putting this up here as an initial proposal of what I think would be a good starting point for a UI for multiple column sorting.
The idea is that each time you click on a header, it toggles between three states, "ascending", "descending", and "cleared". The order in which multiple columns are sorted are denoted by a small number next to the up/down icon. So (for example) to sort by ('last_name', 'first_name')
, you click on the "last name" column, then the "first name" column. To change that to ('-last_name', 'first_name')
you would click once more on the "last name" column. Clicking "last name" again would set the ordering to just ('first_name')
.
This patches cleanly against r10843. Testers are always appreciated.
Attachments (10)
Change History (53)
by , 15 years ago
Attachment: | django_admin_multisort.patch added |
---|
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 15 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:4 by , 14 years ago
Cc: | added |
---|
comment:5 by , 14 years ago
Cc: | added |
---|
comment:6 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:7 by , 14 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
#11695 and #4926 were closed as duplicates.
The reason why the admin currently forces the use of just one field for ordering is merely for a lack of a better UI. And I think that the UI approach suggested here is the right one.
The patch needs to be updated to work with current trunk. It is also important to make it work if the ordering is changed via overriding the ModelAdmin queryset (see #7309).
comment:8 by , 14 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:9 by , 14 years ago
Easy pickings: | unset |
---|
I've updated this patch to apply to trunk, but only looked at it lightly. It still needs review, and tests, and maybe a glace at the CSS to make sure it'll work right cross-browser.
comment:10 by , 14 years ago
After playing with this a bit more, I'm not entirely sold on the UX... clicking around I can't quite predict what my next click will do. I Like being able to sort by multiple fields, but it's very weird to me that a click on a second column doesn't resort.
I'm not sure about a better approach, but this a bit confusing as it stands. It *looks* OK, but it doesn't *act* right, at least to me.
comment:11 by , 14 years ago
After some discussion on IRC, I have an alternate proposal:
If the developer specifies multiple ordering fields in your ModelAdmin
, then you get multiple ordering with the UI as-is here. But if a user clicks on a header, he's back back to single ordering as it worked before. Additionally, there's a "restore default ordering" button (please, come up with a better name!) somewhere to bring back the original ordering.
This doesn't solve the issue completely, but it *does* fix the bug (multiple orderings are ignored), and it does it in a way that a future patch could add a more complex UI, perhaps cribbed from something like Excel or Numbers.
comment:12 by , 14 years ago
I'm happy with Jacob's proposal above. The main problem I have with the current patch is the header clicking.
To clarify Jacob's coment about the UI, if multiple column ordering is specified in ModelAdmin
, the small-numbers-in-column-headers should be displayed. A "Reset sort ordering" button would need to be displayed for any model with a specified ordering
, multicolumn or not. I'm not sure whether this button deserves a whole horizontal bar to itself below the search and admin actions, on one hand it is a waste of vertical space, on the other hand it feels kind of arbitrary to shoehorn it in with either search or admin actions.
comment:13 by , 14 years ago
Cc: | added |
---|
comment:14 by , 14 years ago
There is another approach illustrated in http://tablesorter.com/docs/#Examples
Basically, column headers only toggle between ascending/descending. You can then *select* multiple sorting columns by clicking the headers while pressing the shift key. The selected headers then have a different background colour. The big difference between this example and the admin is that in the admin the page would likely refresh after each click, which may be a bit disturbing. But maybe that could still work. What do you think?
comment:15 by , 14 years ago
Is there a good reason not to follow existing conventions here? This might be my Windows background speaking but I always found Explorer's UI for this to be intuitive, discoverable and non-intrusive.
- Maintain a list of sort columns. i.e. [date, name, size]
- Every click on a column heading adds that item to the front of the queue (and removes it if it's already in the list)
From the users point of view, every click sorts in the way they would expect. Power users quickly learn that the previous sort is also preserved as the secondary sorting column. The UI only needs to indicate the primary sorting column.
comment:16 by , 14 years ago
Cc: | added |
---|
comment:17 by , 14 years ago
@andybak: I think that suggestion is good in general, but has some problems:
1) You pay for additional sorting in terms of performance even if you don't need it. This is probably a minor consideration, I would live with it.
2) There needs to be a way to reset to the original sorting. If, on first view, it is sorted by something sensible (e.g. last name, first name), the user might change it, but then want to restore the original sorting without having to go back n pages and start again (especially if they have also done other filtering and searching). But with this proposal, it might not be obvious to them how to restore the sorting.
I think we do need the 'restore original sorting' link - but I think we need to find some UI space for it.
A suggestion for UI space: on the primary sorting column, we currently have an up/down arrow indicating sorting. We could add an additional icon to the left of this one which shows the advanced controls when you click on it. With andybak's suggestion, the advanced controls could just have:
- a list showing the currently used sort fields
- a 'restore original sort' link.
We could make it more advanced if necessary, but that would suffice for now.
I can't think of anywhere else to find space for this in the UI, because of the way we build up rows of controls i.e. search bar, date hierarchy bar, actions bar, and I really don't want a whole new bar just for this.
comment:18 by , 13 years ago
OK, Andy's sorting mechanic sounds fine (and better than the previous idea).
I also like Luke's UI suggestion -- it's more compact, and it doesn't feel shoehorned into the UI somewhere, plus it provides some explicit information about the sorting. No need to make it any more complex / advanced than this for now.
comment:19 by , 13 years ago
I've added a patch which is nearly there. You'll need the icon_primary_sort.png in django/contrib/admin/media/img/admin/
I tried it without the numbers indicating sorting, but it was more confusing, so I left them in.
Some things that need fixing:
- A better icon! I'm not brilliant at icon design at the best of times, and this was not the best of times...
- The styling on the popup could probably be improved.
- There is a tricky bug that is apparent if you, for example, go to the admin page for auth.Group and click on the header - you get a crash. The root cause seems to be that
__str__
is being passed in somewhere, and it can't be resolved as a field. I've run out of time to debug this. - Possibly we'd want to change the popup so it showed '(ascending)' or '(descending)' next to each field. That would require adding in some translated strings somewhere, and also passing the information through from admin_list.py to the template.
I'm not sure what we want to do in terms of tests. The vast majority of this has to be tested by hand, or using some proper browser based testing like Selenium, for which we've got no infrastructure. The kind of unit tests we can write are a pain in the neck to write and add little value.
by , 13 years ago
Attachment: | 11868_luke3.diff added |
---|
Fixed some existing tests, and bugs with handling columns invalid for sorting
follow-up: 21 comment:20 by , 13 years ago
milestone: | → 1.4 |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
The bug I mentioned turned out to be a separate bug in existing functionality, fixed in [16312]. I've uploaded an updated patch that addresses various issues neglected in my first patch.
Regarding showing 'ascending' or 'descending' on the popup, I don't know if I can come up with a method that is i18n friendly. I don't think we can assume that doing:
* Field name (ascending)
is going to look right in all languages (does Arabic use brackets like that, for instance?). So it's probably best to leave that for now.
I've have tried and failed to write some sensible tests. You can write tests that test the implementation, rather than functionality, but that is not useful. If you try to test functionality i.e. click link on first column, then second column, you get into a regex parsing nightmare, or a DOM traversal nightmare. To code "click on the link inside the header than contains the text 'Field name'" succinctly and robustly, you need a library/infrastructure that Django's test suite just doesn't provide. I already found in [16313] that I had to remove a test because of fragility.
So, I'm removing 'needs tests'. Just awaiting some UI review now, and a better icon - help appreciated!
comment:21 by , 13 years ago
Replying to lukeplant:
Regarding showing 'ascending' or 'descending' on the popup, I don't know if I can come up with a method that is i18n friendly. I don't think we can assume that doing:
* Field name (ascending)is going to look right in all languages (does Arabic use brackets like that, for instance?). So it's probably best to leave that for now.
Something like this would work (pseudo code, assuming there would be a way to get the order in the templates):
{% if header.asc %} {% blocktrans with fieldname=header.fieldname %}{{ fieldname }} (ascending){% endblocktrans %} {% else %} {% blocktrans with fieldname=header.fieldname %}{{ fieldname }} (descending){% endblocktrans %} {% endif %}
or (but this is less preferable since 'ascending' and 'descending' might be translated differently depending on the context):
{% blocktrans with fieldname=header.fieldname order=header.order %}{{ fieldname }} ({{ order }}){% endblocktrans %}
Either way the translators can manipulate the actual translation and RTL languages can handle it on their own.
Which reminds me, you might want to extend rtl.css
to cope with the right aligning.
comment:22 by , 13 years ago
This is looking good. Could we get away by displaying the up/down arrow icons in the popup as well, instead of "(ascending) / (descending)" text? If not then the first approach mentioned by Jannis would be best.
For the icon, a cog might possibly do (it's commonly used for displaying extra options in a popup menu). You can find a sample in http://www.famfamfam.com/lab/icons/silk/
As for the tests, I think we should still add some to verify that Model.Meta.ordering
and ModelAdmin.ordering
are fully honoured.
Also, I haven't tested this thoroughly yet, but how would this all fit with custom querysets (see #7309)?
comment:23 by , 13 years ago
Regarding RTL, I discovered #16144, and need to fix that first. (Review appreciated on that, to ensure I haven't got a broken setup here).
comment:24 by , 13 years ago
OK, thanks for input from everyone. I believe I've got a patch that addresses all concerns raised:
- I've gone with Jannis' suggestion regarding 'ascending/descending', because I don't think using icons in that popup is really helpful.
- I've fixed it in RTL mode
- I've added a basic test for multiple sorting. It doesn't test the URLs generated for each column header, for the reasons of parsing HTML that I described earlier.
- Added tests for
ModelAdmin
and Meta being respected, in the right order. - #7309 - my patch now addresses that too, also adding tests for it - thanks for pointing out Julien.
- I found a suitable cog image, good suggestion. The one I've used is perhaps a tad big, but I couldn't find one smaller, and it does give a better target for hitting than the dot I had before, and makes the feature more obvious.
BTW, I used a '.' instead of ',' in the query string, because the former is not escaped, and so looks much nicer in the address bar.
I'll commit shortly. Any other tweaks to UI styling can then be done more easily.
comment:26 by , 13 years ago
Ough, that icon is rather aweful! It really blows up the table header vertically, didn't you find a better image?
comment:27 by , 13 years ago
Hmm, in the testing I thought it left the table header the same size.
Finding a nice icon is tricky - you can't just resize them at this small size, and it also has to be gif, for transparency support on IE6.
Here are a couple of alternatives I found/made:
http://lukeplant.me.uk/uploads/cog_icon.gif
http://lukeplant.me.uk/uploads/cog_icon2.gif
They are a bit smaller and hard to hit with the mouse, but look a bit nicer. I made them a bit wider than need to be, to increase the chance of hitting them.
follow-up: 29 comment:28 by , 13 years ago
Is it just me or can I no longer filter by just the second column in the display, reset always resets to the first one, clicking on the second column then gives me a sorting by two columns…
comment:29 by , 13 years ago
Replying to apollo13:
Is it just me or can I no longer filter by just the second column in the display, reset always resets to the first one, clicking on the second column then gives me a sorting by two columns…
I presume that "Reset sorting" will reset the default sorting, which means it's no longer possible to sort by a single column other than the default one. Perhaps there should also be an option "Clear sorting" that would clear all sorting options including the default one.
Now that I've used it a bit more, I think one more way to improve the user experience is to also add a "Close" link/button in the popup, and also to make the popup close when pressing the "Esc" key.
I like the two small icons that you've suggested, Luke. I've got a slight preference for "cog_icon2.gif" as it's a bit more subtle.
comment:30 by , 13 years ago
As I mentioned to Florian on IRC, there isn't really a need to sort on one column, as long as you can sort by your primary column. But I agree it could be a distraction/annoyance.
A 'clear sorting' option would be tricky in terms of how it would work, and easy to confuse with 'reset sorting'. An alternative is that in the popup there is an option to explicitly remove a field from the sorting. We could also add an option to explicitly toggle any of the existing fields. I'm not sure if these are refinements we can live without for now. With the current code, however, it isn't that hard to add them, now that I've got this in my head.
comment:32 by , 13 years ago
I don't want to keep messing around with this, so I'll try to get feedback on this latest patch and then hopefully be done with it.
On top of the committed changes, this patch:
- Implements the ESC handling and Cancel button suggested by Julien
- Adds 'toggle' and 'remove' buttons for each field in the sorting list, thereby allowing to sort on just the desired columns (or no sorting,
- Switches to a table layout for the popup. This has the consequence of simplifying the i18n issues
- Adds styling consistent with the date picker popup (actually, got this almost for free with the table markup).
comment:33 by , 13 years ago
Wow, this is awesome! The popup looks great to me. I've found two issues though:
- The popup doesn't show up when clicking on the cog in IE7 -- however this might be due to my crappy IE7 emulator on the Mac. It'd be worth testing on proper IE 6&7 setups.
- I'm not sure if it's directly related to this patch, but the headers break when using a custom
ModelAdmin
method with theadmin_order_field
attribute. See the attached screenshot. Here's the code I've used:
from django.db import models class Person(models.Model): name = models.CharField(max_length=100) age = models.PositiveIntegerField() is_employee = models.NullBooleanField()
from django.contrib import admin from models import Person class PersonAdmin(admin.ModelAdmin): list_display = ('name', 'age', 'is_employee', 'colored_name') def colored_name(self, obj): return '<span style="color: #%s;">%s</span>' % ('ff00ff', obj.name) colored_name.allow_tags = True colored_name.admin_order_field = 'name' admin.site.register(Person, PersonAdmin)
by , 13 years ago
Attachment: | django-changelist-header-bug.png added |
---|
Changelist headers break when using custom ModelAdmin method with admin_order_field
comment:36 by , 13 years ago
There was already a bug with multiple columns that have the same sort field, but the changes made it much worse. Fixing it properly requires one bit of code getting uglier, but another getting nicer. Done in [16319]
I've tested in proper IE6 and 7, and ironed out bugs (with some hacks). It's not exactly the same as other browsers, but good enough. Since these were major bugs, I just committed straight away - [16321]. Testing in IE8 and 9 would be appreciated - I don't have VMs for them.
I've also committed the more advanced UI - [16320]
comment:37 by , 13 years ago
If the window is narrow enough that the added sort icons don't fit without wrapping the column header, I'm seeing a darker gray bar at the top of the header (see attached screenshot)...is that intentional? It looks odd to me...
comment:38 by , 13 years ago
I can't reproduce that with most recent Firefox, Opera or Chrome. It looks like you are using Chrome - what version? I tested Chrome 11 and 13, and couldn't reproduce that issue. That makes it seem like an old Chrome bug that is fixed in recent versions. I'm not sure we'll be able to do anything about that, unless there is an obvious work around.
comment:39 by , 13 years ago
Cancel that - with a larger icon I can reproduce this, on several browsers, so it is an issue.
comment:40 by , 13 years ago
Hmm, screenshot was from Chromium 12.0.742.68 (86550) Ubuntu 10.04 but I am seeing similar behavior on Firefox (both 3.6.17 and 4.0.1). On further playing with it the behavior is not as consistent as I first thought -- the headers can wrap without the bar appearing but then when I add another sort column the bar may appear, it may disappear if I remove one or may not...I've not yet figured out the pattern to what makes it show up.
comment:41 by , 13 years ago
Fixed in [16322]. The bug was really an old one - the existing way of changing the colour of those headers was a hack that worked if the cell wasn't too tall. I've done it properly now.
Also, this is related to the following tickets; #389, #2234, #4926, #5673, #8682