Skip to content

Fix YUIDocs for transition#169

Merged
rwjblue merged 2 commits intoemberjs:masterfrom
Gaurav0:transition_doc
Aug 26, 2015
Merged

Fix YUIDocs for transition#169
rwjblue merged 2 commits intoemberjs:masterfrom
Gaurav0:transition_doc

Conversation

@Gaurav0
Copy link
Contributor

@Gaurav0 Gaurav0 commented Aug 26, 2015

This is an attempt to fix issue emberjs/ember.js#12142

Copy link
Member

Choose a reason for hiding this comment

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

These declarations have to come after the description (otherwise YUIDoc will not parse the description at all).

So it should be:

/**
  Description and examples here

  <yuidoc statements here>
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@rwjblue
Copy link
Member

rwjblue commented Aug 26, 2015

👍

rwjblue added a commit that referenced this pull request Aug 26, 2015
@rwjblue rwjblue merged commit 04b27c9 into emberjs:master Aug 26, 2015
@bendemboski
Copy link

Question here -- is the urlMethod property considered private? I see that it can be set via the method() method, but it's not declared outside the constructor or declared public or private. Do we want to call it out explicitly as either public or private?

The reason I ask is because I'm trying to work around an issue where calling transitionTo() from within a route's beforeModel hook sometimes leaves that route's URL in the nav history (e.g. if the user navigated to that URL from outside the application, so it was already in the history when the transition started and aborting the transition leaves it there), and sometimes doesn't (e.g. if the route was activated from within the app via a {{link-to}} or transitionTo(), in which case the URL isn't yet in the history, so aborting it prevents it from being added). It looks like checking the transition's urlMethod for null is a reliable way to distinguish between these two cases, but I'd feel better about it if urlMethod were documented as a public property.

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Mar 30, 2016

@bendemboski I would consider all undocumented methods and properties private. Your issue can be solved by using Route#replaceWith instead of Route#transitionTo

@bendemboski
Copy link

@Gaurav0 yes, I definitely consider anything that is undocumented to be private. This pull request changed the status of an entire class from being undocumented to being documented (which is awesome -- thank you, by the way), and flagged a bunch of its properties and methods as either public or private. I'm pointing out that one of its properties, urlMethod, was not flagged as either public or private, suggesting that we specify it one way or the other, and asking if we are willing to make it public. I could just submit a pull request and discuss it there, but thought I'd ask here first to see if anybody has any opinions.

As far as my specific issue, simply switching to Route#replaceWith doesn't work -- see this issue. The suggested workaround is to use transition.sequence, which is also not document and appears to only be used for logging and won't work in unit tests without extra code. I noticed that transition.urlMethod seems to be a better indicator, as it is what the router actually uses to decide whether to modify the browser history, and is more of a direct test of whether Route#replaceWith will do what I want, or will corrupt my navigation history.

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Mar 30, 2016

urlMethod does not have any documentation itself, so I would treat it as private.

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.

3 participants