Docs fixes #3200

paulfalgout · 2016-09-26T10:47:45Z

So far I've done three things:

Fixed doc menus
Fixed some documentation errors mainly in general functions and added a couple of missing API.
Added a doc linking common class functions to the general documentation

I'm going to keep going on this PR, but at any point in the commits, if this is ready to merge, I can pull out the later commits or whatever to get it in and continue on another PR.

paulfalgout · 2016-09-26T12:22:29Z

I should also note that I've been updating the live examples as I've gone.. soo... if there's something wrong with an update we'll have to revert.. but I'm not currently sure of a better way to do this.

scott-w

This looks like it could be merged as it stands to me.

scott-w · 2016-09-27T14:53:30Z

docs/marionette.behavior.md

@@ -231,6 +229,39 @@ var ToolTip = Mn.Behavior.extend({
 });
 ```

+#### Behavior Defaults
+`defaults` can be a `hash` or `function` to define the default options for your `Behavior`. The default options will be overridden depending on what you set as the options per `Behavior`. (This works just like a `Backbone.Model`.)


For this, I've been referencing ./basics.md#functions-returning-values

scott-w · 2016-09-27T14:57:17Z

docs/marionette.behavior.md

+Marionette.Behavior.extend({
+handleDestroyClick: function() {
+this.view.destroy();
+}


Missing indent

scott-w · 2016-09-27T15:07:16Z

docs/marionette.object.md


 ## Destroying A Object

 Objects have a `destroy` method that unbind the events that are directly attached to the
 instance.

-Invoking the `destroy` method will trigger a "before:destroy" event and corresponding
+Invoking the `destroy` method will trigger a `before:destroy` event and corresponding
 `onBeforeDestroy` method call. These calls will be passed any arguments `destroy`


I'd be tempted to chop this bit out and just link through to the trigger binding docs.

yeah.. I think the oddity here worth documenting is anything you pass as arguments to destroy get passed to the destroy events which is unique to destroy.

Fair enough - we should still maintain the link and perhaps add a **Note** here?

cool on the link.. I'll get that added.. what'd be in the **Note** ?

I meant using the **Note** to tell readers that destroy will pass its arguments into its events and this is different to other event handling stuff.

paulfalgout · 2016-09-29T07:38:01Z

Just a progress note.. I'm still looking to review Region, ChildViewContainer, and CollectionView docs, but I feel ok about the rest. I'm not really taking into account too many open issues at the moment.. just going through the docs and fixing issues that stand out to me.

rafde

The fiddles need updating to reflect the changes. That can be done afterwards (update without "Set as Base" is done) but before the docs are live.
Aside from that, 👍

rafde · 2016-09-29T15:58:16Z

docs/marionette.functions.md

+
+var myView = new MyView();
+myView.do('action:one');
+myView.do('action:two');
 ```

 [Live example](https://jsfiddle.net/marionettejs/zzjhm4p1/)


I am guessing the fiddle will get updated later.

I just hadn't saved the fiddle on accident.. I think I've updated most of them already.

paulfalgout · 2016-09-30T07:17:34Z

Ok mainly down to regions now..

paulfalgout · 2016-09-30T16:31:15Z

I believe #3172 is resolved here

scott-w

Awesome stuff. It's getting late so I'll have a proper look at the latter files in the docs later.

scott-w · 2016-10-02T20:10:09Z

docs/events.md

+  childViewEvents: function() {
+    return {
+      render: this.onChildRendered
+    }


Is this necessary or can we just link through to the relevant documentation?

good catch.. just copy-pasta'd didn't notice the 2nd part

scott-w · 2016-10-02T20:12:31Z

docs/events.md

+
+  // Events hash defines local event handlers that in turn may call `triggerMethod`.
+  events: {
+    'click .button': 'onClickButton'


It might be better to use an explicit name such as showMessages here?

I just moved this from elsewhere.. although I typically follow naming my function names after the event in this way. Then inside those functions it will procedurally call whatever needs to happen even if it is only showMessages. Typically I follow this because when in the future the button needs to show message and then do something else do you rename the function? This is also the functionality behind triggerMethod so I find it consistent. Right now, I suppose, my naming structure can be dangerous because of auto-proxying, but hopefully that can be disabled soon.

scott-w · 2016-10-02T20:16:27Z

docs/events.md

+
+  childViewTriggers: {
+    'show:message': 'child:show:message',
+    'submit:form': 'child:submit:form'


This could be a bit confusing as it's close enough to the default childview:show:message events. I find a totally different convention is usually more helpful for new developers to separate the concepts when building applications.

Yeah I'm all for that. for event names I pretty much follow https://github.com/jmeas/backbone.class-event-spec

However perhaps we can make a second pass on this stuff? I didn't really change anything style-wise I don't think.. most of this was changing incorrect or misleading things.

Yeah no worries, I might have time over the next week or two to run through it all.

scott-w · 2016-10-02T20:17:48Z

docs/marionette.behavior.md


 Behaviors, like views, have a `ui` attribute that can reference and cache DOM
 elements, just as in the `View`. For more detail, see the
-[`ui` documentation for views](./marionette.view.md#organising-your-view).
+[`ui` documentation for views](./marionette.view.md#organizing-your-view).


Good spot, my Britishness must have slipped in :P

:-) This was changed elsewhere by somebody, but this link got left out

scott-w · 2016-10-02T20:27:35Z

docs/marionette.functions.md

+
+var MyObject = Mn.Object.extend({
+  initialize: function(){
+    this.triggerMethod('baz');


Is this supposed to be this.triggerMethod('foo', 'baz');

scott-w · 2016-10-02T20:32:18Z

docs/marionette.view.md

+
+### Setting a `template` to `false`
+
+Setting the `template` to `false` allows for the view to create all of the bindings and trigger all view events without re-rendering the el of the view. Any other falsy value will throw an exception.


I'd be tempted to wrap the last sentence in italics: *Any other falsy value will throw an exception.*

coveralls · 2016-10-03T07:40:57Z

Coverage remained the same at 100.0% when pulling 65d5eca on paulfalgout:docs-fixes into 43847a1 on marionettejs:master.

paulfalgout · 2016-10-04T12:30:24Z

One more todo: need to look at the view add:region remove:region events and API.

paulfalgout · 2016-10-04T17:04:55Z

Ok I think everything is on here @marionettejs/marionette-core
I believe this is blocking the doc work for v3.1

I'm sure there's still more to do such as add more fiddles or additional adjustments.. but aside from errors, I'd recommend getting this in and making adjustments on subsequent PRs.

coveralls · 2016-10-04T17:06:15Z

Coverage remained the same at 100.0% when pulling 6c9230c on paulfalgout:docs-fixes into 43847a1 on marionettejs:master.

Added a common file linking shared instance functions to the global documentation

Partially resolves marionettejs#3162

Link to function returning values and fix the js example formatting

Also swapped some “ for ‘ for consistency.

coveralls · 2016-10-10T07:05:52Z

Coverage remained the same at 100.0% when pulling cb3f706 on paulfalgout:docs-fixes into 38dc9ca on marionettejs:master.

denar90 · 2016-10-10T07:23:25Z

@paulfalgout thanks)

* Update basics getOptions * Fix Application doc menu * Fix approuter doc menu * Update behavior docs * Fix templating doc menu * Fix common Mn functions Added a common file linking shared instance functions to the global documentation * Fix application docs view lifecycle link * Fix Object docs * Fix viewlifecycle docs * Add lifecycle state methods and prerendered view documentation Partially resolves marionettejs#3162 * behaviors doc fixes Link to function returning values and fix the js example formatting * Fix destroying an object docs * Update view and templating docs * Fix advanced collectionview doc * CollectionView doc fixes * Revision fixes Also swapped some “ for ‘ for consistency. * Add undocumented view function * Updating Region documentation * Add missing API to region docs # Conflicts: # docs/marionette.region.md # docs/marionette.view.md

paulfalgout force-pushed the docs-fixes branch 4 times, most recently from 7febe1d to 744bf5c Compare September 27, 2016 08:01

scott-w approved these changes Sep 27, 2016

View reviewed changes

paulfalgout mentioned this pull request Sep 28, 2016

Release v3.1 #3192

Closed

paulfalgout force-pushed the docs-fixes branch 7 times, most recently from 4dbd45c to f199d80 Compare September 29, 2016 07:33

paulfalgout force-pushed the docs-fixes branch 2 times, most recently from 7c41e12 to 19ccbe9 Compare September 29, 2016 07:41

rafde approved these changes Sep 29, 2016

View reviewed changes

paulfalgout force-pushed the docs-fixes branch 9 times, most recently from 10faef0 to b10578a Compare September 30, 2016 07:07

scott-w reviewed Oct 2, 2016

View reviewed changes

scott-w approved these changes Oct 8, 2016

View reviewed changes

paulfalgout added 19 commits October 10, 2016 15:55

Update basics getOptions

e1749a7

Fix Application doc menu

5f6a9ec

Fix approuter doc menu

227b661

Update behavior docs

5fdd902

Fix templating doc menu

37b0ea4

Fix common Mn functions

b3900eb

Added a common file linking shared instance functions to the global documentation

Fix application docs view lifecycle link

fbdba52

Fix Object docs

4661a82

Fix viewlifecycle docs

d0dadd2

Add lifecycle state methods and prerendered view documentation

ea72916

Partially resolves marionettejs#3162

behaviors doc fixes

1097986

Link to function returning values and fix the js example formatting

Fix destroying an object docs

9b79277

Update view and templating docs

3b1b09c

Fix advanced collectionview doc

3fd90ef

CollectionView doc fixes

97a2d7c

Revision fixes

c62f071

Also swapped some “ for ‘ for consistency.

Add undocumented view function

4a74e76

Updating Region documentation

2f84992

Add missing API to region docs

cb3f706

paulfalgout force-pushed the docs-fixes branch from 6c9230c to cb3f706 Compare October 10, 2016 06:57

denar90 approved these changes Oct 10, 2016

View reviewed changes

denar90 merged commit 08e92f0 into marionettejs:master Oct 10, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs fixes #3200

Docs fixes #3200

paulfalgout commented Sep 26, 2016

paulfalgout commented Sep 26, 2016

scott-w left a comment

scott-w Sep 27, 2016

scott-w Sep 27, 2016

scott-w Sep 27, 2016

paulfalgout Sep 28, 2016

scott-w Sep 28, 2016

paulfalgout Sep 28, 2016 •

edited

scott-w Sep 28, 2016

paulfalgout commented Sep 29, 2016

rafde left a comment

rafde Sep 29, 2016

paulfalgout Sep 29, 2016

paulfalgout commented Sep 30, 2016

paulfalgout commented Sep 30, 2016

scott-w left a comment

scott-w Oct 2, 2016

paulfalgout Oct 3, 2016

scott-w Oct 2, 2016

paulfalgout Oct 3, 2016

scott-w Oct 2, 2016

paulfalgout Oct 3, 2016

scott-w Oct 3, 2016

scott-w Oct 2, 2016

paulfalgout Oct 3, 2016

scott-w Oct 2, 2016

paulfalgout Oct 3, 2016

scott-w Oct 2, 2016

coveralls commented Oct 3, 2016

paulfalgout commented Oct 4, 2016

paulfalgout commented Oct 4, 2016

coveralls commented Oct 4, 2016

coveralls commented Oct 10, 2016

denar90 commented Oct 10, 2016


		### Setting a `template` to `false`

		Setting the `template` to `false` allows for the view to create all of the bindings and trigger all view events without re-rendering the el of the view. Any other falsy value will throw an exception.

Docs fixes #3200

Docs fixes #3200

Conversation

paulfalgout commented Sep 26, 2016

paulfalgout commented Sep 26, 2016

scott-w left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulfalgout Sep 28, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulfalgout commented Sep 29, 2016

rafde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulfalgout commented Sep 30, 2016

paulfalgout commented Sep 30, 2016

scott-w left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 3, 2016

paulfalgout commented Oct 4, 2016

paulfalgout commented Oct 4, 2016

coveralls commented Oct 4, 2016

coveralls commented Oct 10, 2016

denar90 commented Oct 10, 2016

paulfalgout Sep 28, 2016 •

edited