jgallen23 / fidel

A basic javascript controller
MIT License
36 stars 4 forks source link

v3 minor re-write #3

Closed jgallen23 closed 10 years ago

jgallen23 commented 11 years ago

Goals:

Examples:

//define - returns Class (same as declare in v2).  Only way to set up a plugin moving forward
var Slider = fidel.define('Slider', {
  init: function() {
    //this.el
  }
});

//init class by var - JS Style
var slider = new Slider({ el, options});

//or init class from views object - JS Style
var slider = new fidel.views.Slider({ el, options });

//init via jQuery (must include optional jquery.fidel.js file)
$().slider({ options });

jQuery Plugin

Questions

@dawnerd @Belelros - would love to get your input on this.

dawnerd commented 11 years ago

I like fidel.define a lot more than declare. Have you thought of maybe doing fidel.view('Slider', {...}) instead? I think that might make more since if you're putting it into fidel.views. That would also open the possibility of a fidel module that isn't a view, like a service.

I'd keep the backwards compatibility for now with a console.info in there.

Antonio-Laguna commented 11 years ago

I personally prefer define and modules. The view thing is more like be using a MV* framework which (for now) it's not the case. Unless you want to go in that direction, I think modules has more sense.

About backwards compatibility, it really depends on this things (for me):

If there is so many people using Fidel, it may be worth to add backwards compatibility. If changes are big, there are no reasons to keep only that for compatibility. Keep in mind that nowadays it's damn easy to stick with one version in particular due to semver.

jgallen23 commented 11 years ago

Thanks for the feedback. I agree with the MV* stuff. I don't want fidel to turn into a competitor of backbone, etc. It should be kept as a lightweight module controller for non-SPA sites and a helper for writing jQuery plugins.

@Belelros Tough to say how many use fidel. I'm sure it's been tough to adopt for others since there are no docs. For v3, the main goal is cleaning up the api (stuff above) and documentation. Not looking to add any more major features - mainly because I can't think of any others to add.

jgallen23 commented 11 years ago

lets go with define and modules

@Belelros you want to create a v3 branch and start updating the tests and lib?

Antonio-Laguna commented 11 years ago

Sure!

Antonio-Laguna commented 11 years ago

Can you add me as contributor or should I fork it?

jgallen23 commented 11 years ago

done

jgallen23 commented 11 years ago

Note: need to think about how defaults across all instances are handled (currently jquery.fn.plugin.defaults)

jgallen23 commented 10 years ago

@Belelros have you had a chance to do any work on this?

Antonio-Laguna commented 10 years ago

Have you had any time to check on this?

jgallen23 commented 10 years ago

I will today

jgallen23 commented 10 years ago

perfect!

jgallen23 commented 10 years ago

now just need to add some tests for fidel.modules.ModuleName.

We also need to think about how to handle defaults. something like: fidel.modules.ModuleName.defaults.prop = 123 or fidel.defaults.ModuleName.prop = 123

jgallen23 commented 10 years ago

few more tasks:

Antonio-Laguna commented 10 years ago

So, there is a test which checks that the view is actually being saved into the fidel.modules namespace. What else do you want to check there?

Also I'd say that it makes more sense to keep the defaults in the module namespace.

I'll take a look into the other stuff

Antonio-Laguna commented 10 years ago

What's the real purpose of the defaults being exposed like that? Updating them later on?

jgallen23 commented 10 years ago

So you can set an option for all new instances. For example, if you wanted all popboxes to have direction up, but don't want to keep setting it on every instance

Antonio-Laguna commented 10 years ago

I'm not sure about what do you exactly mean with the last point.

Antonio-Laguna commented 10 years ago

I'm not sure what went wrong with fatjs but I keep receiving:

Warning: Task "jshint:tools" not found. Use --force to continue.

jgallen23 commented 10 years ago

hmm. see if there is a grunt/jshint.yaml in the fatjs folder and let me know what it looks like

Antonio-Laguna commented 10 years ago

It's there, is really really weird...

options:
  reporter: <%= jshintStylish %>
  jshintrc: <%= fatjsPath %>/.jshintrc
tools:
  - Gruntfile.js
  - bower.json
  - package.json
lib:
  - lib/**/*.js
test:
  - test/*.js
jgallen23 commented 10 years ago

what happens when you just run grunt jshint? I can take a look today

Antonio-Laguna commented 10 years ago

Same thing... I tried to put some console.logs into the tasks but everything seem to be working fine. I've reinstalled fatjs and everything and saw nothing strange.

I don't know what's wrong really :(

jgallen23 commented 10 years ago

I would try npm prune. I just ran on a clean branch and it worked fine. Here's the results:

[jga@Gregs-Mac-mini.local] ~/code/fidel [v3]: grunt
Running "jshint:tools" (jshint) task

✔ No problems

Running "clean:dist" (clean) task
Cleaning dist...OK

Running "jshint:lib" (jshint) task

✔ No problems

Running "bower:main" (bower) task
Skipping jquery (/Users/jga/code/fidel/bower_components/jquery/jquery.js)
dist/_bower.js written

Running "concat:lib" (concat) task
File "dist/fidel.js" created.

Running "concat:full" (concat) task
File "dist/fidel.full.js" created.

Running "clean:bower" (clean) task
Cleaning dist/_bower.js...OK

Running "jshint:test" (jshint) task

test/fidel.test.js
  line 10   col 12  'fidel' is not defined.
  line 10   col 32  'viewObj' is not defined.
  line 18   col 3   'teardown' is not defined.
  line 26   col 30  'fidel' is not defined.
  line 30   col 30  'fidel' is not defined.
  line 53   col 20  'viewObj' is not defined.
  line 57   col 20  'fidel' is not defined.
  line 61   col 7   'fidel' is not defined.
  line 69   col 7   'fidel' is not defined.
  line 82   col 36  'Fidel' is not defined.
  line 324  col 7   'Fidel' is not defined.
  line 328  col 17  'fidel' is not defined.
  line 337  col 7   'Fidel' is not defined.
  line 342  col 17  'fidel' is not defined.
  line 333  col 12  'v' is defined but never used.
  line 347  col 12  'v' is defined but never used.

test/fixture.js
  line 1    col 12  'viewObj' is defined but never used.
  line 45   col 30  'e' is defined but never used.

test/jquery.test.js
  line 9    col 12  'fidel' is not defined.
  line 9    col 39  'viewObj' is not defined.
  line 20   col 3   'teardown' is not defined.
  line 30   col 45  'viewObj' is not defined.

✖ 22 problems

Warning: Task "jshint:test" failed. Use --force to continue.

Aborted due to warnings.
Antonio-Laguna commented 10 years ago

That didn't work and I tried that before.

However, removing the node_modules folder and installing again made the trick, I suspect that previous dependencies were matching fatjs ones and because of that they weren't downloaded and hence, failing.

How do I tame that to get the globals without errors?

jgallen23 commented 10 years ago

try something like: /*global fidel, teardown */ in the test file. Let me know if that works

Antonio-Laguna commented 10 years ago

That worked fine. Now facing more issues... How do we tell fatjs to also take care of fidel.jquery.js? Right now it's not including the right files and hence, tests fails.

jgallen23 commented 10 years ago

just link to the lib/jquery.js file for now

Antonio-Laguna commented 10 years ago

Ok!

Antonio-Laguna commented 10 years ago

Anything else you want me to look within this?