Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adding HtmlTable.Resize #189

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Owner

anutron commented Jan 5, 2011

This is the work of marcusmc. It's tested and works, though I have a feeling that the code style might not fit in w/ More.

@anutron anutron Adding HtmlTable.Resize
* Rebased the work of marcusmc
f9b65a6
Contributor

fat commented Mar 15, 2011

Hm.. personally, i think this Class.refactor pattern is bogus.

Say i have 3 tables on a page... 1 table I want regular, 1 zebra, and 1 sortable, resizable, and selectable.

Correct me if I'm wrong, but that means that i have to not only include 5 .js files, but furthermore each file rewrites my HtmlTable class object into an event larger beast of a class. My regular table instance in this case would be loaded with tons of methods and logic that are useless to it...

Instead, what i would like to see is an HtmlTable.js and HtmlTable.extras.js. Thennn... in HtmlTable.extras.js instead of using class refractor, and options to switch how code was executed, we would have mixin classes for zebra, sortable, selectable, and maybe resizable... that way as a developer i could be more flexible with my table development...

Contributor

fat commented Apr 1, 2011

I don't like this personally, for the reasons above.

Owner

timwienk commented Apr 1, 2011

As far as I understand, Class.refactor is for quick monkey-patches. I agree it shouldn't be used like this. But not using it here will mean inconsistency with the other HtmlTable plugins, removing it from the others as well breaks compatibility. So it's not a dealbreaker for me.

I do agree though, we should change this to either mixins or just extensions of the HtmlTable class some time in the future.

Owner

anutron commented Apr 11, 2011

so, I'm going to close this pull request for now. the code isn't mine and it's not as well written as the rest of our stuff.

The refactor pattern was used here because mixins don't give you the ability to call methods of the "super" class. And consider what happens when you have instances of the class in every combo. You'd end up with:

HtmlTable
HtmlTable.Sortable
HtmlTable.SortableZebra
HtmlTable.Zebra
HtmlTable.Resizable
HtmlTable.ResizableZebra
HtmlTable.ResizableSortableZebra

etc.

I'm curious how you guys would avoid this problem. Having mixins is nice, but you still need a base class that mixes in the stuff you need.

Another alternative is just one big-ass file with all the functionality in it to support these features, something else I'm not crazy about...

@anutron anutron closed this Apr 11, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment