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

Allow use of custom class names for ordered columns through header at… #329

Merged
merged 6 commits into from
May 23, 2016

Conversation

michaelmob
Copy link
Contributor

…tributes

@michaelmob
Copy link
Contributor Author

michaelmob commented May 18, 2016

I wasn't entirely sure if the TableBase constructor is totally off-limits by adding header_attrs.
Nonetheless, I've added it but it could be changed without too much issue.

I decided that putting header attributes into Column wouldn't make much sense considering someone would have no need for separate ordering attributes for different columns; I believe they would want to change it for the entire table globally.

If opinions differ, give me a better direction to go and I'll attempt to improve on it.

I'll create Docs if it is decided that this implementation is in the right direction for this project. Thanks!

@michaelmob
Copy link
Contributor Author

Usage would be:

class MyTable(tables.Table):
    [...]
    Meta:
        header_attrs = {
            'orderable': 'sortable',  # Custom ordered class
            'descending': 'descend',  # Custom descending class
            'ascending': 'ascend'  # Custom ascending class
        }

@jieter
Copy link
Owner

jieter commented May 19, 2016

Your point about changing it for a complete table makes sense. I didn't mean to forbid these kind of args to the table constructor, but as said, it's already crowded so we need to consider things carefully.

thetarkus added 2 commits May 19, 2016 21:09
Remove header_attrs from table constructor
Remove attrs comment?
Rename blocklist to blacklist
New assert, verify table's class does not get put into th's class
@michaelmob
Copy link
Contributor Author

michaelmob commented May 20, 2016

How about this approach.. The most recent commits are what this is.

Instead of having header_attrs in the constructor, a th key is put into the attrs for the table or for the column. Then within that th key, you can put any attribute and value except for _ordering which is reserved for column ordering class names.

For example:

Table(queryset, attrs={
    'th': {
        'class': 'custom-header-class',
        '_ordering': {
            'orderable': 'sort',
            'ascending': 'ascending',
            'descending': 'descending'
        }
    }
})

This will also work for td on table and on a column. If a column provides a separate attrs for td then that will take priority over the table's.

Table(queryset, attrs={
    'td': {
        'class': 'custom-header-class'
    }
})

Hope this made sense. If you're into this approach then I'll write some documentation. (In a more clear manner and more in-depth.)

@jieter
Copy link
Owner

jieter commented May 20, 2016

@thetarkus thanks for trying another approach.

What I like about this is that it's more along the lines for what we have in place for column attributes in Column, what I don't like is the slightly more complex syntax (and mixing ordering settings with direct html attributes). I guess it's a trade-off between number of arguments and complexity of the arguments.

Let's go with this and see how it works out.

@jieter
Copy link
Owner

jieter commented May 23, 2016

@thetarkus looks good, merging

@jieter jieter merged commit d9ebdfc into jieter:master May 23, 2016
jieter added a commit that referenced this pull request May 23, 2016
@michaelmob michaelmob deleted the header-attributes branch May 24, 2016 05:20
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