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

[BC] Fix the default name of index column to follow DT syntax. #1882

Merged
merged 1 commit into from
Oct 30, 2018

Conversation

sharifzadesina
Copy link
Contributor

@sharifzadesina sharifzadesina commented Oct 17, 2018

This is a small PR that changes the name of the index column to follow DT syntax.
DT syntax for special columns is like: DT_NameName
But the name of the index column was DT_Name_Name
I don't know this is a breaking change or no cause I only changed the default value.

@yajra
Copy link
Owner

yajra commented Oct 18, 2018

I think this is breaking change but necessary since I think we can consider this as a bug fix.

On the other hand, It will not break only if they have published the config. Or just revert the old name by publishing and editing the value on config file.

@yajra yajra changed the title Changes the default name of index column to follow DT syntax [BC] Fix the default name of index column to follow DT syntax. Oct 18, 2018
@yajra
Copy link
Owner

yajra commented Oct 18, 2018

BTW, I think this should also be implemented on https://github.com/yajra/laravel-datatables-html/blob/3.0/src/Html/Builder.php#L472.

@yajra
Copy link
Owner

yajra commented Oct 18, 2018

Will wait until weekend to see if some users have comments on this. If no reactions, I'm ok to release this next week. Thanks!

@yajra yajra added the bug label Oct 18, 2018
@yajra yajra merged commit 6acb385 into yajra:8.0 Oct 30, 2018
jaydons added a commit to jaydons/laravel-datatables-html that referenced this pull request Oct 30, 2018
@sharifzadesina sharifzadesina deleted the 8.0-index-column-fixed branch October 30, 2018 13:48
yajra added a commit to yajra/laravel-datatables-docs that referenced this pull request Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants