search

pjekel / cbtree

The Dijit Tree with Multi State Checkboxes, project code 'cbtree' , is a highly configurable dojo/dijit tree with support for multi-state checkboxes or third party widgets capable of presenting a so-called 'checked' state.
Other
75 stars 34 forks source link

made classes truly anonymous; relative paths in the sample #2

Closed vvoovv closed 12 years ago

vvoovv commented 12 years ago

I also suggest to rename Samples -> demos (low case!) index.html -> tree01.html

pjekel commented 12 years ago

Hi Vladimir,

First, with regards to the repository name my preference would be to rename it to 'cbt' (CheckBox Tree), I setup 'TreeWithCheckboxes' locally in a separate directory as I'm not comfortable with git and I just wanted to make sure it would not mess-up my own environment. I will try to rename it accordingly.

Second, making the paths relative is a good idea, I will also rename the 'sample' directory to demo.

Third, I'm not sure about making all the classes anonymous. I might be missing the point but as a result you now have to pass 'twc' as an argument to every function. In addition, this is neither the way dojo or dijit is currently declaring their classes. I do understand it is allowed from an AMD perspective but again it is not consistent with the current dojo and dijit implementation.

BTW: Have you tested any of your changes?

Thanks for your help,

Peter

-----Original Message----- From: Vladimir Elistratov [mailto:reply@reply.github.com] Sent: Thursday, March 15, 2012 5:47 AM To: pjekel Subject: [TreeWithCheckboxes] made classes truly anonymous; relative paths in the sample (#2)

I also suggest to rename Samples -> demos (low case!) index.html -> tree01.html

You can merge this Pull Request by running:

git pull https://github.com/vvoovv/TreeWithCheckboxes master

Or you can view, comment on it, or merge it online at:

https://github.com/pjekel/TreeWithCheckboxes/pull/2

-- Commit Summary --

-- File Changes --

M CheckBox.js (2) M Samples/index.html (40) M StoreModel.js (4) M Tree.js (17)

-- Patch Links --

https://github.com/pjekel/TreeWithCheckboxes/pull/2.patch https://github.com/pjekel/TreeWithCheckboxes/pull/2.diff

Reply to this email directly or view it on GitHub: https://github.com/pjekel/TreeWithCheckboxes/pull/2

vvoovv commented 12 years ago

Yes, I tested the changes with Samples/index.html

dojo and dijit modules currently declare namespace since they have to be backwards compatible with older versions. All new packages (e.g. dgrid, https://github.com/SitePen/dgrid) are being developed anonymous. After dojo 2.0 explicit namespaces won't be available. I'd suggest to refactor the current demo so it's not necessary to pass twc as a function parameter.

As for the repo name I'd vote for cbtree (tree is preserved is the name). Obviously, the final decision (cbt or cbtree) is up to you.

Here is info how to deal with pull requests: http://help.github.com/send-pull-requests/

pjekel commented 12 years ago

Crap, I just changed it cbt, Oh well, I change it one more time: cbtree it is...

-----Original Message----- From: Vladimir Elistratov [mailto:reply@reply.github.com] Sent: Thursday, March 15, 2012 10:18 AM To: pjekel Subject: Re: [TreeWithCheckboxes] made classes truly anonymous; relative paths in the sample (#2)

Yes, I tested the changes with Samples/index.html

dojo and dijit modules currently declare namespace since they have to be backwards compatible with older versions. All new packages (e.g. dgrid, https://github.com/SitePen/dgrid) are being developed anonymous. After dojo 2.0 explicit namespaces won't be available. I'd suggest to refactor the current demo so it's not necessary to pass twc as a function parameter.

As for the repo name I'd vote for cbtree (tree is preserved is the name). Obviously, the final decision (cbt or cbtree) is up to you.

Here is info how to deal with pull requests

Reply to this email directly or view it on GitHub: https://github.com/pjekel/TreeWithCheckboxes/pull/2#issuecomment-4520381