continuouscalendar / jquery-continuous-calendar

Date picker and range selector with scrollable months instead of paged (the only right way to do it)
http://continuouscalendar.github.io/jquery-continuous-calendar/
85 stars 35 forks source link

Abstract templating #73

Closed fiskus closed 10 years ago

fiskus commented 11 years ago

Hello again. I replaced hardcoded strings with html by abstract Template methods. It is better because these methods could be redefined and replaced, for example, by precompiled underscore templates. I splitted all templates into two groups. First contains templates with classnames and selectors that are present in other code (besides CSS), and the second contains templaes that can be totally changed. Simple examples and tests are also provided.

eeroan commented 11 years ago

Overall I think this is an improvement. It also makes code more readable. I will comment on few minor things. You can then make a new pull request

fiskus commented 11 years ago

Thanks, I will implement this and your future notes (if you will find something) in a few days.

eeroan commented 11 years ago

Hi,

Could it be possible to prevent from overriding these private templates, if that's the case? Could they could be in a separate module that would be forced to use?

On 10.9.2013, at 13.29, Maxim Chervonny notifications@github.com wrote:

Hello again. I replaced hardcoded strings with html by abstract Template methods. It is better because these methods could be redefined and replaced, for example, by precompiled underscore templates. I splitted all templates into two groups. First contains templates with classnames and selectors that are present in other code (besides CSS), and the second contains templaes that can be totally changed. Simple examples and tests are also provided.

You can merge this Pull Request by running

git pull https://github.com/fiskus/jquery-continuous-calendar abstract-templating Or view, comment on, or merge it at:

https://github.com/reaktor/jquery-continuous-calendar/pull/73

Commit Summary

add abstract templating Merge branch 'master' of github.com:reaktor/jquery-continuous-calendar into abstract-templating add bodyRow to abstract templates add inner wrapper to abstract template add abstract templating for RangeEvents, SingleDateEvents and jquery.plugin fix tests ability to redefine template add example of redefined template let it be public (could be redefined) and private templates add description and example for abstract templating add jsdoc to template add example for templating with underscore library tests for Template.js File Changes

M site/example.html (20) M site/example.js (16) M src/main/CalendarBody.js (77) M src/main/RangeEvents.js (12) M src/main/SingleDateEvents.js (7) A src/main/Template.js (212) M src/main/jquery.continuousCalendar.js (23) A src/test/Template.spec.js (42) M src/test/testSuite.js (3) Patch Links:

https://github.com/reaktor/jquery-continuous-calendar/pull/73.patch https://github.com/reaktor/jquery-continuous-calendar/pull/73.diff

Eero Anttila eea@iki.fi +358 50 359 0079

fiskus commented 11 years ago

Well, technically there no realy private and public templates, they all can be overrided. I divide them only for one reason: "private" templates have classnames that already used in code. So if someone override template with that classname he could get broken code, because searching of selectors would return nothing. I think that these selectors should be moved into options too, and then all templates would be 'public'. So I don't know what should I do in this situation, if you think that should be two modules, I will divide Template.js into TemplatePublic.js and TemplatePrivate.js, and merge them only when selectors would be as options.

eeroan commented 11 years ago

Ok. We can do this in baby steps. Let's keep them in same module as they where. I'm trying figure out other choises

On 11.9.2013, at 23.38, Maxim Chervonny notifications@github.com wrote:

Well, technically there no realy private and public templates, they all can be overrided. I divide them only for one reason: "private" templates have classnames that already used in code. So if someone override template with that classname he could get broken code, because searching of selectors would return nothing. I think that these selectors should be moved into options too, and then all templates would be 'public'. So I don't know what should I do in this situation, if you think that should be two modules, I will divide Template.js into TemplatePublic.js and TemplatePrivate.js, and merge them only when selectors would be as options.

— Reply to this email directly or view it on GitHub.

Eero Anttila eea@iki.fi +358 50 359 0079

eeroan commented 11 years ago

One option would be removing all the selectors from the code and instead pass the objects when needed. BTW, what's the use case for this possibility to replace markup? Just curious.

fiskus commented 11 years ago

First: current markup is not good for some reasons. For example, it has common classnames like .label or .month or .week or .separator. Regular website can already have those classnames defined in CSS-files. And someone could want to override these classnames with other, with prefix and with dashes instead camelCase. Also it is dificult to stylize tables, so someone could want to override table cells with divs.

Second: templates could be overriden with the same templates but generated with another (with more speed) template. As I know, precompiles underscore template has the most speed.

Third: posibillity to insert additional elements. For example, I want to replace icon-element by button that looks like input element for regular range calendar. Or I want to insert words "start" and "end" inside startRange cell and endRange cell.

These is just my real needs, someone could has more.

eeroan commented 11 years ago

Thanks for the answer. If the class names are too general, perhaps they should be renamed by default.

I totally agree with the third argument. After this improvement getCalendarContainerOrCreateOne() could be simplified. It would always be created based on template.

On 12.9.2013, at 11.11, Maxim Chervonny notifications@github.com wrote:

First: current markup is not good for some reasons. For example, it has common classnames like .label or .month or .week or .separator. Regular website can already have those classnames defined in CSS-files. And someone could want to override these classnames with other, with prefix and with dashes instead camelCase. Also it is dificult to stylize tables, so someone could want to override table cells with divs.

Second: templates could be overriden with the same templates but generated with another (with more speed) template. As I know, precompiles underscore template has the most speed.

Third: posibillity to insert additional elements. For example, I want to replace icon-element by button that looks like input element for regular range calendar. Or I want to insert words "start" and "end" inside startRange cell and endRange cell.

These is just my real needs, someone could has more.

— Reply to this email directly or view it on GitHub.

Eero Anttila eea@iki.fi +358 50 359 0079

eeroan commented 10 years ago

I'll close this for now.