Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Unit tests from API Documentation #49

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

leobalter
Copy link
Contributor

Fixes #45

This is based on #46 and the target of this PR is to identify the API features and build the documentation and tests for that.

@leobalter leobalter changed the title API Documentation [WIP] API Documentation Feb 19, 2015
@leobalter
Copy link
Contributor Author

The documentation will lead the unit tests, so that's why I need answers for the questions I did at #46:

  • What $("#editor").Arte() should return? (Considering #editor is a valid element for ArteJS)
  • It's an object, not an object with the previous jQuery selection, right?
  • What are the properties and methods from the returned object?
  • What are the type of the expected properties?
  • How and when should I call those methods?
  • What they return?

@leobalter leobalter changed the title [WIP] API Documentation [WIP] Unit tests from API Documentation Feb 19, 2015
@gshenar
Copy link
Contributor

gshenar commented Feb 23, 2015

  • $("#editor").Arte() should return a jQuery object containing a TextArea object for each element that was matched in the selector. For example in the case that you call $("#editor").Arte() it will create a TextArea for the #editor element on the page if one hasn't been created yet and return it.
  • If you call $("#editor").Arte(options) it will do the same but use the options that you passed in instead of the default options
  • If you call $("#editor").Arte(string Command) for example "bold" it will call the bold command on the TextArea and return the return value from that command (currently returns undefined - we might want to change this to just return the TextArea)

The TextArea object has alot of methods available to it. Some of them are defined directly here in TextArea
These have return values which we are currently working to document.

More methods available on the TextArea object are defined here - Commands
These commands are called in order to modify the html of the rich text area. They currently don't return anything.

@leobalter
Copy link
Contributor Author

  1. jQuery object containing a TextArea object, currently it is not returning a jQuery object, but probably this TextArea object, containing 2 properties making a reference to the former jQuery object.
  2. $("#editor").Arte(options), the options are not documented. I don't even know what are the default options.
  3. $("#editor").Arte(string Command), This is probably documented here, but it would be better if we have a description of each command. The current documentation shows only a list of commands.

These commands are called in order to modify the html of the rich text area. They currently don't return anything.

Actually the commands are returning the jQuery object (not the TextArea object). As seen at ced8f6e

@chriskwan
Copy link
Contributor

@leobalter, what happens is that the command itself does not return anything (returns undefined) but it gets put in an array and wrapped in a jQuery object (see here). So what we get is a jQuery object that only contains undefined. This might not be the API we want (it might be better to have the command return the original object so we can chain) but that's what's happening now.

@leobalter
Copy link
Contributor Author

that's what's happening now

And that's what we don't need to document. We don't need to say what the program is currently doing, but what it should be doing. That's where we're going to know where to do the code refactoring.

@chriskwan
Copy link
Contributor

I committed some initial commented files. @gshenar and I are actively working on this and will be adding more as we go.

@leobalter
Copy link
Contributor Author

@chriskwan, your last commits mixed some tabs and spaces on the indentation. I fixed them but is good to keep track of them. Maybe setting your editor to show invisible characters might help.


arte = this.element.Arte();

arte.text("foo");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not possible, break the tests.

How can I add text via JS in my Arte elements?

@leobalter
Copy link
Contributor Author

I reverted the new unit tests as I'm writing then along with a huge refactoring on another branch. I'll squash this revert commit before merging.

@leobalter leobalter mentioned this pull request Feb 25, 2015
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve demo and documentation site
3 participants