fixes #2532134 in datatable sort #281

Merged
merged 4 commits into from Jan 11, 2013

Conversation

Projects
None yet
6 participants
@clanceyp
Contributor

clanceyp commented Sep 25, 2012

Fixed case sensitive sort order for string columns in datatable. Added test to cover data table containing Date, Number and String data types with case sensitive sort available by setting a caseSensitive param to true on the table column.

Setting a column with caseSensitive param to true e.g...

{key:"Company", label:"Sortable string (case sensitive)", sortable:true, caseSensitive:true}

Will force case sensitive sorting, the default being case insensitive.

fixes #2532134 Added sort test to cover Date, Number and String data …
…types with case sensitive sort available by setting a caseSensitive param to true on the table column.
@lsmith

This comment has been minimized.

Show comment
Hide comment
@lsmith

lsmith Sep 25, 2012

Contributor

Love it! Thanks for taking care of this. Since the _compare function will be executed a lot, especially on large data sets, I'd prefer to replace Y.Lang.isString(...)_calls withtypeof checks to avoid the function hops.

Otherwise, could you add an entry to the HISTORY.md file?

LGTM.

@ericf, should this be merged to master or 3.x?

Contributor

lsmith commented Sep 25, 2012

Love it! Thanks for taking care of this. Since the _compare function will be executed a lot, especially on large data sets, I'd prefer to replace Y.Lang.isString(...)_calls withtypeof checks to avoid the function hops.

Otherwise, could you add an entry to the HISTORY.md file?

LGTM.

@ericf, should this be merged to master or 3.x?

@clanceyp

This comment has been minimized.

Show comment
Hide comment
@clanceyp

clanceyp Sep 25, 2012

Contributor

Sure, one other thing, is there anywhere I can document the caseSensitive
param?
It's something I have trouble with, finding out what params are supported
by the code.

Thanks Patrick

PS I'll update to...
if (!cs && typeof(aa) === "string" && typeof(bb) === "string") {...}

If that's ok?

On Tue, Sep 25, 2012 at 4:47 PM, Luke Smith notifications@github.comwrote:

Love it! Thanks for taking care of this. Since the compare function will
be executed a lot, especially on large data sets, I'd prefer to replace
Y.Lang.isString(...)
calls with typeof checks to avoid the function hops.

Otherwise, could you add an entry to the HISTORY.md file?

LGTM.

@ericf https://github.com/ericf, should this be merged to master or 3.x?


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8859126.

Contributor

clanceyp commented Sep 25, 2012

Sure, one other thing, is there anywhere I can document the caseSensitive
param?
It's something I have trouble with, finding out what params are supported
by the code.

Thanks Patrick

PS I'll update to...
if (!cs && typeof(aa) === "string" && typeof(bb) === "string") {...}

If that's ok?

On Tue, Sep 25, 2012 at 4:47 PM, Luke Smith notifications@github.comwrote:

Love it! Thanks for taking care of this. Since the compare function will
be executed a lot, especially on large data sets, I'd prefer to replace
Y.Lang.isString(...)
calls with typeof checks to avoid the function hops.

Otherwise, could you add an entry to the HISTORY.md file?

LGTM.

@ericf https://github.com/ericf, should this be merged to master or 3.x?


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8859126.

ref #2532134 Changed Y.Lang.isString() to typeof() === 'string' to im…
…prove performance, and added comment to component history.
@lsmith

This comment has been minimized.

Show comment
Hide comment
@lsmith

lsmith Sep 25, 2012

Contributor

You can document it on the user guide located in src/datatable/docs/index.mustache

Specifically, I'd add a note here: http://yuilibrary.com/yui/docs/datatable/#sorting and the config entry here: http://yuilibrary.com/yui/docs/datatable/#column-config

Contributor

lsmith commented Sep 25, 2012

You can document it on the user guide located in src/datatable/docs/index.mustache

Specifically, I'd add a note here: http://yuilibrary.com/yui/docs/datatable/#sorting and the config entry here: http://yuilibrary.com/yui/docs/datatable/#column-config

@ericf

This comment has been minimized.

Show comment
Hide comment
@ericf

ericf Sep 25, 2012

Member

@lsmith This seems like it should go into master. I can merge it in after the final sign off.

Member

ericf commented Sep 25, 2012

@lsmith This seems like it should go into master. I can merge it in after the final sign off.

@ericf

This comment has been minimized.

Show comment
Hide comment
@ericf

ericf Sep 25, 2012

Member

@clanceyp Have you signed the YUI CLA? If not it would be great if you could:
http://yuilibrary.com/contribute/cla/

Also, let me know once you've signed it. Thanks.

What Is the Purpose of the CLA?

The CLA ensures that everyone who submits a work of authorship to the YUI Library is contributing work that is their own or for which they can authoritatively speak. This protects the tens of thousands of developers who use YUI in their daily work, all of whom rely on YUI's BSD license to appropriately cover their use of the library.

The CLA does not transfer title or copyright of your contributed work to Yahoo!. It merely guarantees that you approve the use of your work within YUI and by those who use the library under the terms of its license.

Member

ericf commented Sep 25, 2012

@clanceyp Have you signed the YUI CLA? If not it would be great if you could:
http://yuilibrary.com/contribute/cla/

Also, let me know once you've signed it. Thanks.

What Is the Purpose of the CLA?

The CLA ensures that everyone who submits a work of authorship to the YUI Library is contributing work that is their own or for which they can authoritatively speak. This protects the tens of thousands of developers who use YUI in their daily work, all of whom rely on YUI's BSD license to appropriately cover their use of the library.

The CLA does not transfer title or copyright of your contributed work to Yahoo!. It merely guarantees that you approve the use of your work within YUI and by those who use the library under the terms of its license.

@clanceyp

This comment has been minimized.

Show comment
Hide comment
@clanceyp

clanceyp Sep 27, 2012

Contributor

I did sign one (paper copy) and send it in to the address mentioned in the
document a couple of weeks ago.

I never heard anything back though so not sure if your office
ever received it.

Thanks Patrick

On Tue, Sep 25, 2012 at 11:00 PM, Eric Ferraiuolo
notifications@github.comwrote:

@clanceyp https://github.com/clanceyp Have you signed the YUI CLA? If
not it would be great if you could:
http://yuilibrary.com/contribute/cla/

Also, let me know once you've signed it. Thanks.

What Is the Purpose of the CLA?

The CLA ensures that everyone who submits a work of authorship to the YUI
Library is contributing work that is their own or for which they can
authoritatively speak. This protects the tens of thousands of developers
who use YUI in their daily work, all of whom rely on YUI's BSD license to
appropriately cover their use of the library.

The CLA does not transfer title or copyright of your contributed work to
Yahoo!. It merely guarantees that you approve the use of your work within
YUI and by those who use the library under the terms of its license.


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8871907.

Contributor

clanceyp commented Sep 27, 2012

I did sign one (paper copy) and send it in to the address mentioned in the
document a couple of weeks ago.

I never heard anything back though so not sure if your office
ever received it.

Thanks Patrick

On Tue, Sep 25, 2012 at 11:00 PM, Eric Ferraiuolo
notifications@github.comwrote:

@clanceyp https://github.com/clanceyp Have you signed the YUI CLA? If
not it would be great if you could:
http://yuilibrary.com/contribute/cla/

Also, let me know once you've signed it. Thanks.

What Is the Purpose of the CLA?

The CLA ensures that everyone who submits a work of authorship to the YUI
Library is contributing work that is their own or for which they can
authoritatively speak. This protects the tens of thousands of developers
who use YUI in their daily work, all of whom rely on YUI's BSD license to
appropriately cover their use of the library.

The CLA does not transfer title or copyright of your contributed work to
Yahoo!. It merely guarantees that you approve the use of your work within
YUI and by those who use the library under the terms of its license.


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8871907.

@clanceyp

This comment has been minimized.

Show comment
Hide comment
@clanceyp

clanceyp Sep 27, 2012

Contributor

I've added a couple of entries to the docs. If you could take a look when
you get a sec.

Thanks Patrick

On Tue, Sep 25, 2012 at 7:27 PM, Luke Smith notifications@github.comwrote:

You can document it on the user guide located in
src/datatable/docs/index.mustache

Specifically, I'd add a note here:
http://yuilibrary.com/yui/docs/datatable/#sorting and the config entry
here: http://yuilibrary.com/yui/docs/datatable/#column-config


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8864790.

Contributor

clanceyp commented Sep 27, 2012

I've added a couple of entries to the docs. If you could take a look when
you get a sec.

Thanks Patrick

On Tue, Sep 25, 2012 at 7:27 PM, Luke Smith notifications@github.comwrote:

You can document it on the user guide located in
src/datatable/docs/index.mustache

Specifically, I'd add a note here:
http://yuilibrary.com/yui/docs/datatable/#sorting and the config entry
here: http://yuilibrary.com/yui/docs/datatable/#column-config


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8864790.

@jenny

This comment has been minimized.

Show comment
Hide comment
@jenny

jenny Sep 28, 2012

Member

Hi Patrick,

We've gone digital with our CLA. Would you mind resubmitting it here: http://yuilibrary.com/contribute/cla/

Thanks!

Member

jenny commented Sep 28, 2012

Hi Patrick,

We've gone digital with our CLA. Would you mind resubmitting it here: http://yuilibrary.com/contribute/cla/

Thanks!

@clanceyp

This comment has been minimized.

Show comment
Hide comment
@clanceyp

clanceyp Sep 28, 2012

Contributor

All done. (Is that new? I'm sure it wasn't there a couple of weeks ago. : )

Thanks P

On Fri, Sep 28, 2012 at 8:23 AM, Jenny Donnelly notifications@github.comwrote:

Hi Patrick,

We've gone digital with our CLA. Would you mind resubmitting it here:
http://yuilibrary.com/contribute/cla/

Thanks!


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8966615.

Contributor

clanceyp commented Sep 28, 2012

All done. (Is that new? I'm sure it wasn't there a couple of weeks ago. : )

Thanks P

On Fri, Sep 28, 2012 at 8:23 AM, Jenny Donnelly notifications@github.comwrote:

Hi Patrick,

We've gone digital with our CLA. Would you mind resubmitting it here:
http://yuilibrary.com/contribute/cla/

Thanks!


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8966615.

@ghost ghost assigned ericf Sep 28, 2012

@clanceyp

This comment has been minimized.

Show comment
Hide comment
@clanceyp

clanceyp Oct 13, 2012

Contributor

Did this get signed off at all?

thanks patrick

On Tue, Sep 25, 2012 at 10:58 PM, Eric Ferraiuolo
notifications@github.comwrote:

@lsmith https://github.com/lsmith This seems like it should go into
master. I can merge it in after the final sign off.


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8871861.

Contributor

clanceyp commented Oct 13, 2012

Did this get signed off at all?

thanks patrick

On Tue, Sep 25, 2012 at 10:58 PM, Eric Ferraiuolo
notifications@github.comwrote:

@lsmith https://github.com/lsmith This seems like it should go into
master. I can merge it in after the final sign off.


Reply to this email directly or view it on GitHubhttps://github.com/yui/yui3/pull/281#issuecomment-8871861.

@lsmith

This comment has been minimized.

Show comment
Hide comment
@lsmith

lsmith Nov 7, 2012

Contributor

Ship it!

Contributor

lsmith commented Nov 7, 2012

Ship it!

@ghost ghost assigned apipkin Dec 4, 2012

@yuibuild yuibuild merged commit 85557a2 into yui:master Jan 11, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment