Skip to content
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

2202 multicolumn sort interface #130

Open
wants to merge 19 commits into
base: 7.0
Choose a base branch
from

Conversation

leckronz
Copy link

Enables tiered sorting.
User can add and remove fields from sort, and change direction between ascending and descending within the sort.
Sort rank is indicated numerically in the column header.
If no columns are selected by the user and a default sort field is being used, the default sort field is indicated with highlighting and ascending/descending class.

seeker/views.py Outdated Show resolved Hide resolved
seeker/views.py Outdated
sr_label = (' <span class="sr-only">(%s)</span>' % sort) if sort else ''
if self.field_definition:
span = '<span title="{}" class ="fa fa-question-circle"></span>'.format(self.field_definition)
else:
span = ''
html = '<th class="%s"><a href="?%s" title="Click to sort %s" data-sort="%s">%s%s %s</a></th>' % (cls, q.urlencode(), next_sort, q['s'], self.header_html, sr_label, span)
html = '<th class="%s"><a href="?%s" title="Click to %s" data-sort="%s">%s%s %s</a></th>' % (cls, q.urlencode(), next_sort, q['s'], self.header_html, sr_label, span)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this to an f-string so it's a little more readable?

html = f'<th class="{ cls }"><a href="?{ q.urlencode() }" title="Click to { next_sort }" data-sort="{ q["s"] }">{ self.header_html }{ sr_label } { span }</a></th>'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most current version of the code uses format_html() for this line and removes mark_safe() when passing back the html at the end of the function as per #136. If I were to use an f string like in your comment, I would also need to go back to using mark_safe(), correct?
Would this be undermining your change in the linked PR 136?

seeker/views.py Outdated
'order': 'desc' if desc else 'asc',
'missing': missing,
}
sort: sort_querydict
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, I think we can revert the changes to this function

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct, I've reverted them in the most recent commit

seeker/views.py Outdated
@@ -1525,11 +1539,16 @@ def render_results(self, export):

# Finally, grab the results.
sort = self.get_sort_field(columns, self.search_object['sort'], display)

default_sort_field = ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, let me know if you figure out what this was used for so we can determine if it's still needed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default_sort_field holds the value of the field upon which to sort when no other columns have been selected, or all other columns that were previously sorted upon are deselected.
So when we first navigate to a page we are sorting by default_sort_field, say field A. If we were to sort by field B and field C, and then remove field B and field C from the sort, the results would revert to being sorted by field A.

@leckronz leckronz changed the base branch from 6.0 to 7.0 January 14, 2022 20:35
…at_html and returning html instead of mark_safe(html) for column headers
…ation of sort rank/sort order when only one field is being sorted upon]
…ne whether or not to use the default sort field, account for .label as part of column field name
…ing as the first sort-upon field, causing a sort rank to display
…label the next sorting direction if client is not set up to use multisort for AdvancedSeeker
…onary rather than list of dictionaries) to be non-breaking, initial commit for SeekerView
…header(). Builds a querystring/href for each column that handles adding sort fields, changing sort field direction, and removing sort fields.
@@ -23,9 +23,9 @@
{% endif %}
{% for col in display_columns %}
{% if use_wordwrap_header %}
{% seeker_column_header col results %}
{% seeker_column_header col results sort_descriptor_list %}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort_descriptor_list is passed to column header() to determine if there's a default column being sorted upon (so that it can be indicated)

@@ -88,33 +88,69 @@ def header(self):
if not self.sort:
return format_html('<th class="{}">{}</th>', cls, self.header_html)
q = self.view.request.GET.copy()
field = q.get('s', '')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the existing behavior of SeekerView -- passing the sort information through the querystring, without modifying the context.

sort_dict = {}
for s in sort:
sort_dict.update(self.apply_sort_descriptor(s))
return sort_dict
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During a meeting we changed sort_descriptor() so that it would return a list of dictionaries for the Elasticsearch DSL sort to use (unpacked). I changed it back to returning a single dictionary so that it wouldn't be breaking for certain sites using SeekerView that are calling sort_descriptor().

cls = '{}_{}'.format(self.view.document._doc_type.name, self.field.replace('.', '_'))
cls += ' {}_{}'.format(self.view.document.__name__.lower(), self.field.replace('.', '_'))
if self.model_lower:
cls += ' {}_{}'.format(self.model_lower, self.field.replace('.', '_'))
if not self.sort:
return format_html('<th class="{}">{}</th>', cls, self.header_html)
is_multicolumn = 'isInitialSort' in self.view.search_object
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_multicolumn checks for a flag in the search object to determine the jquery file passing the search object has been set up to build a search object list. This allows us to determine if "remove from sort" should be an option

# Order of precedence for sort is: parameter, the default from the view, and then the first displayed column (if any are displayed)
sort = sort or self.sort or display[0] if len(display) else ''
if is_initial_sort:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this flag allows us to remove the default sort column (specified in the view) from the sort and to sort on nothing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants