Skip to content

Conversation

@arrowflex
Copy link

fix for #40 - disable sorting of columns.
Added two new attributes:
[DataTablesSortable] - set to false to prevent sorting
[DataTablesVisible] - set to false to make column hidden.

Note: because the bSaveState is set to true in the cshtml file you need to clear the browser cookie/local storage in order for this to work. Reading the dataTables documentation, this seems to be by design.

mcintyre321 added a commit that referenced this pull request Sep 20, 2013
add basic attribute support for visible and sortable - possible fix for ...
@mcintyre321 mcintyre321 merged commit fed44a0 into mcintyre321:master Sep 20, 2013
@notdabob
Copy link
Contributor

notdabob commented Oct 2, 2013

This doesn't seem to work I've tried the DataTablesVisible flag and it compiles fine but the column is always still there.

Has this been tested and confirmed to work?

@mcintyre321
Copy link
Owner

Have you tried clearing cache and local storage? (See earlier comment)

sent from my mobile
On 2 Oct 2013 22:27, "Not Bob" notifications@github.com wrote:

This doesn't seem to work I've tried the DataTablesVisible flag and it
compiles fine but the column is always still there.

Has this been tested and confirmed to work?


Reply to this email directly or view it on GitHubhttps://github.com//pull/46#issuecomment-25578584
.

@notdabob
Copy link
Contributor

notdabob commented Oct 2, 2013

Yeah I cleared both and even tried from private browsing in chrome

@mcintyre321
Copy link
Owner

Dave?

sent from my mobile
On 2 Oct 2013 23:08, "Not Bob" notifications@github.com wrote:

Yeah I cleared both and even tried from private browsing in chrome


Reply to this email directly or view it on GitHubhttps://github.com//pull/46#issuecomment-25581355
.

@arrowflex
Copy link
Author

Hi notdabob,

Sorry you are having problems. We have it installed in three projects and it is working well. I'll try to reproduce the problem.

I have a couple of questions:

Is this a new project/new installation of mvc.datatables or did you update an existing installation of mvc.datatables?

Are you using the fixedcolumn plugin for datatables? It does not play nicely with hidden columns. The hidden columns must not be inside the fixed range.

Are you using the standard mvc.datatables "views/shared/DataTable.cshtml"? (I ask because we use a customised one).

Could you please check DataTable.cshtml and make sure that this line appears:
"aoColumnDefs" : @Html.Raw(Model.ColumnDefsString)

It should be under "fnServerData" like this:

        "fnServerData": function(sSource, aoData, fnCallback) {
            $.ajax({
                "dataType": 'json',
                "type": "POST",
                "url": sSource,
                "data": aoData,
                "success": fnCallback
            });
        },
        "aoColumnDefs" : @Html.Raw(Model.ColumnDefsString)
         @Html.Raw(!string.IsNullOrWhiteSpace(Model.JsOptionsString) ? ", " + Model.JsOptionsString : "")
        });

thanks
Dave

@arrowflex
Copy link
Author

Hi notdabob,

one more thing: are you using the mvc.jquery.datatables.templates or just the base package?

Dave

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

Hi guys thanks for the help :)

I'm updating my existing project and we had slightly customized the default datatable.cshtml and that aoColumnDefs line was not in there so I just added it.

The generated output looks like this:
var $table = $('#ViolationTask');
var dt = $table.dataTable({
"bProcessing": true,
"bStateSave": true,
"bServerSide": true,
"bFilter": true,
"sDom": 'T<"clear">lftipr',
"aLengthMenu": [[5, 10, 25, 50, -1], [5, 10, 25, 50, "All"]],
"bAutoWidth": true,
"sAjaxSource": "/ViolationTasks/IndexData", "oTableTools" : { "sSwfPath": "/content/DataTables/extras/TableTools/media/swf/copy_csv_xls_pdf.swf" },
"fnServerData": function(sSource, aoData, fnCallback) {
$.ajax({
"dataType": 'json',
"type": "POST",
"url": sSource,
"data": aoData,
"success": fnCallback
});
},
"aoColumnDefs" : [{"bVisible":false,"aTargets":[2,3]}],

    });

But I'm still seeing all the columns, the index of 2 & 3 in that list is correct though as it should be hiding those 2 columns.

I have both packages mvc.jquery.datatables and the templates package installed

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

I am not using the fixed columns plugin

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

So I pulled the latest off github and ran your example and it seems to be working fine oddly enough on there just not on my actual project unfortunately so I'm going to try removing the packages and cleaning my build etc... what other local storage did I have to wipe?

@arrowflex
Copy link
Author

Excellent! One problem solved :)

So, it looks like it is emitting the correct data. The only reason, that I know of, for datatables to not take notice is if the table config is in local storage (thanks to bSaveState).

Can you try one or all of these please:

1 - Can you clear the browser history (for chrome I use history->clear browsing-> beginning of time -> clear browsing data which is, I know, overkill).

2 - Change bSaveState to false and then clear the browser again. Refresh page, etc.

3 - put [DataTablesSortable(false)] on one of your columns in your model. You should see another entry in aoColumnDefs like this:
{"bSortable":false, "aTargets":[columnid]}

Has the correct column become unsortable or a different one? If they are all sortable then it is most likely the bSaveState.

Cheers
Dave

@arrowflex
Copy link
Author

BTW, it depends on the browser where it stores state - nuke the entire history is my preferred option. For testing turn bSaveState to false.

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

FML I think i found the issue, I had a /Content/DataTables-1.9.4 folder that was still referenced in my bundle config! Cleaning it up now and going to retest :)

@mcintyre321
Copy link
Owner

(Although not the problem on this case) I wonder if a comment on the
attribute warning people about clearing the cache might be useful - it's a
'feature' that is likely to raise further issues.

sent from my mobile
On 3 Oct 2013 18:08, "Not Bob" notifications@github.com wrote:

FML I think i found the issue, I had a /Content/DataTables-1.9.4 folder
that was still referenced in my bundle config! Cleaning it up now and going
to retest :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/46#issuecomment-25638948
.

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

Well that was super not fun but i got mine all working now :)

I was running into problems with the whole DataTablesResults actions as the way the transforms are wired up in the example it appears to be basically pulling the full objects then you're transforming them into the view model which is a huge projection that I'm concerned about with performance. I was able to build a smarter view model and custom get methods to keep the querying down to the minimal fields I need for performance reasons

@mcintyre321
Copy link
Owner

It's meant to be a 2 stage thing - there is meant to be an IQueryable of TQueryModel (which sorting/filtering is carried out against) then an optional extra based transform you can do to render properties of the TQueryModel - I think thats not working right now (it's getting passed to the query which messes with filtering on those properties).

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

Wouldn't that cause the initial stage to pull the full object which would pull all of the fields?

My concern is performance and scale there as many times I'm using data tables to present index views and not necessarily a grid style implementation where I'd want all of the fields.

Can we discuss this new attribute though as I feel like we're doing this kind of inefficient as far as syntax goes...
[DataTablesSortable(false)]
[DataTablesVisible(false)]

I love having these new attributes but wouldn't it be a lot more expandable and cleaner to go more like this:
[DataTables(Sortable = false, Visible = false)]

That would give us an extension point to add in more custom attributes in the long run

@mcintyre321
Copy link
Owner

I'd probably accept that pull request (a combined attribute).

As for performance, the query should be passed to your IQueryableProvider which should take care of pulling back the fields and data you need. If you have some columns/properties that aren't in the data to be displayed, but can be queried on (e.g. Visible=false, Filterable=true), the bit which builds the dynamic select statement could be modified to not include them.

@notdabob
Copy link
Contributor

notdabob commented Oct 3, 2013

I'd like to see some example to help explain how to make what you just described work.

I personally don't have the time right now to side track into forking the repo to make that change but I definitely think it should be done. As like Filterable as you just mentioned would be another great flag to have in there to keep it super easy. As you can tell I'm a big fan of attributes. I have a fork of your repo on mine but no clue how to pull your latest code back into it, I guess I could nuke my repo and rebranch it but that doesn't seem logical.. sorry not a huge git user :P too busy coding trying to make a living

I ran into another fun one that I'm not 100% sure how best to handle... we have a huge products database (>52k records) and if you select all from the pagination option you can run into the: Error during serialization or deserialization using the JSON JavaScriptSerializer. The length of the string exceeds the value set on the maxJsonLength property.

In an ideal situation it'd be nice if it could handle automatically paginating at whatever the max is below that limit but I'm not sure how doable that is.

@arrowflex
Copy link
Author

@notdabob - Glad you got it working. In one project we have removed the "all" pagination option :)

@mcintyre321 - A comment is good idea. I'll add that if no-one else does :)

So, I originally had a single attribute and then I looked at data annotations and EF for inspiration and they seem to keep behaviour separate.

I was also trying to decide if certain attributes could be class level - so (non)Sortable could be (because I have tables where no sorting is allowed) but it wouldn't make sense for visibility to be class level?

That said, it could certainly be improved. I wasn't really happy with the "false" as a parameter.

I'm currently testing a change to the sortable attribute so that I can specify an alternate column . For example:

[DataTablesSortable(Use="Amount")]
string AmountFormatted

[DataTablesVisible(false)]
decimal Amount

It drops more information into the aoColumnDefs field. Works with server side and client side as some of our tables are fully rendered to the browser and datatables take over.

Happy to try out some variations in a branch, if people are happy to take a look at them.

@notdabob
Copy link
Contributor

notdabob commented Oct 6, 2013

Oh I would be super happy with the suggested change for a different sort
column!!!!

We have a ton of date time's in our system and I really prefer to
display them as "5 hours ago" vs an exact date / time but really need to
be able to filter / sort on the underlying datetime column.

On 10/3/13 5:59 PM, Dave Parker wrote:

@notdabob https://github.com/notdabob - Glad you got it working. In
one project we have removed the "all" pagination option :)

@mcintyre321 https://github.com/mcintyre321 - A comment is good
idea. I'll add that if no-one else does :)

So, I originally had a single attribute and then I looked at data
annotations and EF for inspiration and they seem to keep behaviour
separate.

I was also trying to decide if certain attributes could be class level

  • so (non)Sortable could be (because I have tables where no sorting is
    allowed) but it wouldn't make sense for visibility to be class level?

That said, it could certainly be improved. I wasn't really happy with
the "false" as a parameter.

I'm currently testing a change to the sortable attribute so that I can
specify an alternate column . For example:

[DataTablesSortable(Use="Amount")]
string AmountFormatted

[DataTablesVisible(false)]
decimal Amount

It drops more information into the aoColumnDefs field. Works with
server side and client side as some of our tables are fully rendered
to the browser and datatables take over.

Happy to try out some variations in a branch, if people are happy to
take a look at them.


Reply to this email directly or view it on GitHub
#46 (comment).

@mcintyre321
Copy link
Owner

It would be neat to use a name-based convention rather than an attribute - this would allow anonymous types to be used from within views:

queryable<User>().Select(u => new {u.Name, u.Name_Display = "<a href='...'>" + u.Name + "</a>"})

@notdabob
Copy link
Contributor

notdabob commented Oct 7, 2013

Downside to that is people like me who use a real viewmodel instead of
anonymouse types... as that more complicated html you have there imho is
better done in a get method in the view model to separate the concerns
between data & display... just my .02 worth... ideally supporting both
is the best but convention + configuration can equal confusion.

On 10/7/13 9:03 AM, Harry McIntyre wrote:

It would be neat to use a name-based convention rather than an
attribute - this would allow anonymous types to be used from within views:

|queryable().Select(u => new {u.Name, u.Name_Display = "" + u.Name + ""})
|


Reply to this email directly or view it on GitHub
#46 (comment).

@mcintyre321
Copy link
Owner

With the convention you'd be able to do it in either the view itself, the viewmodel or the controller layer. Having the ability to do it in the view layer has some advantages when it comes to using HtmlHelper methods.

@mcintyre321
Copy link
Owner

@arrowflex don't do anything just yet - I have a new version nearly ready which will allow total customisation of column rendering after query execution, and have combined the attributes to make a single one which has properties for Searchable, Visible, DisplayName and Sortable.

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.

3 participants