-
-
Notifications
You must be signed in to change notification settings - Fork 212
-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
Publish Filtering is broken #2091
Comments
Can you check if any other script in the backend if causing errors, John?
I can't reproduce this here.
I'll have to check this. |
I cannot reproduce this either.
This is the only thing I can reproduce and this should not be that hard to fix. |
John, I'm not sure how I can help at the moment. |
I haven't got any console errors from other extensions. I need to pop out to lunch but will hop back on this in an hour. @psychoticmeow it was for something else, will catch you later. @nilshoerrmann I may have to give you access to the system, it's got 148000 entries in the section. |
There's inconsistent behaviour with the clear button. If I click on the filter first to show the dropdown, then click clear, it sometimes works. If I click on the clear button first it never works. There are no errors, and I have deactivated every extenion that applies JS to the publish pages. The publishing never shows the initial count of entries when filtered, but does after you click 'next'. |
I'm starting to think it's the fact there are so many entries that's breaking the JS functionality; Although I'm not sure how. |
Have you tried commenting out the AJAX requestion that fetches the filtered pages? |
Will try that now |
Try to comment out this part and replace it with a submit call: |
Works perfectly now. How can we adjust this then for sections with large datasets? |
Actually, I don't know what the heck is happening, as I haven't recompiled the JS yet. And it is still intermittently failing to clear :( |
@nilshoerrmann those steps completely break it as it now appends loads of query parameters to the URL when you click in the filter box. |
What happens if you replace https://github.com/symphonycms/symphony-2/blob/master/symphony/assets/js/src/symphony.filtering.js#L119-L120 with:
And does your problem occur in all browsers? |
That fixes it, with the only caveat being that it adds the ID being filtered by to the search box. So it is the ajax that can't handle the 148000 entries then, mhhh. |
We are "just" loading the same page using AJAX, replacing the content area dynamically – so that still sounds more like a bug in the script. |
Yes, maybe it is. If I now click the clear button, the same happens; It allows me to edit the filter, but not clear it, so no page refresh happens. I will keep looking at this, but I reckon you'd beat me to an answer. |
I think the problem is related to where the clear anchor sits in the DOM, compared to what JS events are attached to it's parent. I'm not sure from first glance whether that would be a selectize thing or an us thing. |
I'll see if I'll find a minute to look at this early tomorrow morning. |
You're a gent. As per usual. |
John, I currently don't have a clean (function($, Symphony) {
'use strict';
Symphony.Extensions.Filtering = function() {
var filter, fields, comparison, search, rows, maxRows,
comparisonSelectize, searchSelectize, fieldsSelectize;
var init = function(context) {
filter = $(context);
fields = filter.find('.filtering-fields');
comparison = filter.find('.filtering-comparison');
search = filter.find('.filtering-search');
rows = Symphony.Elements.context.find('.filtering-row:not(.template)');
maxRows = Symphony.Elements.context.find('.filtering-row.template .filtering-fields option').length;
// Setup interface
fields.selectize().on('change', switchField);
comparison.selectize().on('change', switchComparison);
search.addClass('init').selectize({
create: true,
maxItems: 1,
sortField: 'text',
render: {
item: itemPreview,
option_create: searchPreview
},
onItemAdd: searchEntries
});
// Store Selectize instances
fieldsSelectize = fields[0].selectize;
comparisonSelectize = comparison[0].selectize;
searchSelectize = search[0].selectize;
// Reduce field options
rows.not(filter).each(reduceFields);
// Remove add button
if(rows.length >= maxRows) {
Symphony.Elements.context.find('.filtering-add').hide();
}
// Highlight filtering
highlightFiltering();
// Clear search
filter.find('.destructor').on('click', clear).on('mouseover mouseout', prepareClear);
// Finish initialisation
search.removeClass('init');
};
var reduceFields = function() {
var row = $(this),
value = row.find('.filtering-fields').val();
fieldsSelectize.removeOption(value);
fieldsSelectize.addItem(Object.keys(fieldsSelectize.options)[0]);
};
var switchField = function() {
// Clear
searchSelectize.clearOptions();
searchSelectize.$control_input.attr('placeholder', Symphony.Language.get('Type to search') + '…');
// Add suggestions
$.ajax({
url: Symphony.Context.get('symphony') + '/ajax/filters/',
type: 'GET',
dataType: 'json',
data: {
handle: fields.val(),
section: Symphony.Context.get('env')['section_handle']
},
success: function(result) {
var contains = false;
// Add options
$.each(result.filters, function(index, data) {
searchSelectize.addOption(data);
if(isNaN(data.value)) {
contains = true;
}
});
// Set comparison mode
if(contains || !result.filters.length) {
comparisonSelectize.setValue('contains');
}
else {
comparisonSelectize.setValue('is');
}
// Refresh suggestions
searchSelectize.refreshOptions(false);
}
});
};
var switchComparison = function() {
if(searchSelectize.getValue() !== '') {
searchEntries();
}
};
var searchPreview = function(item) {
return '<div class="create"><em>' + Symphony.Language.get('Search for {$item}', {item: item.input}) + ' …</em></div>';
};
var itemPreview = function(item, escape) {
return '<div class="item">' + escape(item.text) + '<a href="' + location.href.replace(location.search, '') + '" class="destructor">' + Symphony.Language.get('Clear') + '</a></div>';
};
var searchEntries = function(value, item, exclude) {
if(!search.is('.init')) {
var filters = buildFilters(exclude),
base, url;
// Fetch entries
base = location.href.replace(location.search, '');
url = base + (filters !== '' ? '?' : '') + filters;
// Redirect
window.location.href = url;
}
};
var buildFilters = function(exclude) {
var filters = [];
$('.filtering-row:not(.template)').each(function() {
var row = $(this);
if(row[0] != exclude) {
var fieldVal = row.find('.filtering-fields')[0].selectize.getValue(),
comparisonVal = row.find('.filtering-comparison')[0].selectize.getValue(),
searchVal = row.find('.filtering-search')[0].selectize.getValue(),
filterVal, method;
if(fieldVal && searchVal) {
method = (comparisonVal === 'contains') ? 'regexp:' : '';
filterVal = 'filter[' + encodeURI(fieldVal) + ']=' + method + encodeURI(searchVal);
filters.push(filterVal);
}
}
});
return filters.join('&');
};
var highlightFiltering = function() {
if(Symphony.Elements.breadcrumbs.find('.inactive').length === 0 && location.search.indexOf('filter') !== -1) {
Symphony.Elements.breadcrumbs.append('<p class="inactive"><span>– ' + Symphony.Language.get('filtered') + '</span></p>');
}
};
var prepareClear = function(event) {
if(searchSelectize.$dropdown.is(':hidden')) {
searchSelectize.$dropdown.css('opacity', (event.type === 'mouseover' ? 0 : 1));
}
};
var clear = function(event) {
event.preventDefault();
event.stopPropagation();
var destructor = $(this),
exclude = destructor.parents('.filtering-row')[0];
searchEntries(null, null, exclude);
};
// API
return {
init: init,
clear: clear
};
};
})(window.jQuery, window.Symphony); |
Thanks Nils, I will do in a minute |
@nilshoerrmann that works as expected now but with the last issue of having the ID number in place of the value in the search field when the page refreshes to apply the filter. It's quite quick too, the clear button works, and the pagination is fixed. Is it a big job to change the value in the filter box? Would that be another quick ajax lookup do you think? |
Actually, I don't know what you mean with "having the ID number in place of the value". |
Ah, are you talking about Select Box Links? |
This not as easy as it sounds – that's the reason I opted for fetching the pages using AJAX in the first place. |
Yes, it's an SBL, so when the page reloads, it uses the ID number from the URL instead of the value. |
The thing is that the interface doesn't know anything about the field type – so a numeric value can either be a SBL field or just a number in a text input. Feel free to dig into this – I won't have time to look into this now. Really sorry. |
Oh, no problem Nils, thanks so much for your help so far! |
Clearing and pagination, fixes #2091
I know it was my idea to merge this in, and I was expecting it to be like the old Publish Filtering, not a new (mainly) javascript version, so we could ensure it worked correctly before sprucing it up
It is thoroughly broken now and I have to release this to a client on Friday (tomorrow). Problem is I have no idea where to start with fixing it now as it has completely changed from how it worked originally. I don't want to just have to rely on one person's time to fix all of the UI issues; we should be able to help each other out, but that's another rant.
Issues:
A big issue is that if a filter has been applied, clicking the 'clear' button only visually clears the filter; There is no other effect, so the entries remain filtered.
If I have filtered on one value that has no pagination, then change the filter to another, the pagination element remains missing. If I filter on a value that has pagination, the pagination element has no count of pages or any other value other than the buttons and input box.
I've tagged this as major simply because we have added it as a core feature that just doesn't work. This needs fixing asap.
@nilshoerrmann I really want to be able to help you out here. Where do I start?
The text was updated successfully, but these errors were encountered: