Prezent / prezent-grid-bundle

Integrate prezent-grid in Symfony
MIT License
6 stars 3 forks source link

Inconsistent way of determining the active sortable column by using sent sort column in request #9

Closed seydu closed 6 years ago

seydu commented 6 years ago

A sortable field can define a sort column. When columnA and columnB define field1 as their sort_field, both columns will appear as sorted when you click on one of them. I don't think this is wrong but it can pose UX "ambiguity" (like uses saying "why these two columns are sorted when click on one of them?)

Anyway, by displaying the sort_field attribute in the view we are somehow leaking storage related information in the view (not a big deal, this happens often and is sometimes a compromise that avoids making things more complex! The Law of Leaky Abstractions). We could keep the sort_field attribute but display the column name. The part of the application responsible for passing input data to the storage facility will transform the column name into a field name (by grabbing the field options and reading the sort_field or using any other means). This is better than using the HTTP request object to communicate between the grid and the storage layer.

If we adopt this change it will introduce a BC break but it will be worth it.

sandermarechal commented 6 years ago

I don't quite understand your issue. You can already do this, without any changes. The default value for the sort_field is the column name. It is the application's task to take whatever is in that field and construct some kind of query about it. But this bundle is not tied to any kind of controller or persistence solution.

Did you perhaps mean to report this on prezent/crud-bundle instead?

seydu commented 6 years ago

Thanks for replying.

Yes, I can pretty much do what I want without any change. I had a situation where two columns were showing as sorted. This was because they used the same value for sort_field. That was a mistake that I fixed. But if this was my real intent those two columns would appear simultaneously sorted. Let me try to add a test that illustrates what I tried to explain in the issue (should have started with that!). Thanks again, I really like your approach in designing this tool.

sandermarechal commented 6 years ago

Well, if they use the same value for sort_field, would it not be correct to show both columns as sorted? They are sorted exactly the same way after all. Alternatively, you can give each column a fifferent sort_field value, but just generate the same query for both. They it works exactly as you describe without any changes to this library.

seydu commented 6 years ago

Yes, this may not be a UX issue after all. The most important thing is that developers have the option to not have 2 or more columns sorted simultaneously if they think it is a UX issue for their user base. No need to "prevent" them from doing so. I have a test case here to illustrate what happens when 2 columns use the same value for sort_field.

I will close this issue. Thanks.