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

feat($state): added 'state' to state reload method (feat no.1612) #1809

Merged
merged 1 commit into from
Mar 30, 2015

Conversation

maoryosef
Copy link

  • modiefied options.reload parameter in transitionTo method to accept the
    following:
    {reload : true} will reload all states
    {reload : 'foo.bar'} will reload foo.bar, and any children
    {reload : stateObj} will reload state found in stateObj, and any
    children.
  • added 'state' parameter(string|object) to $state.reload method,
    to be passed to $state.transitionTo method as the reload option.
  • test the $state.reload with state parameter

@christopherthielen
Copy link
Contributor

Thanks, really appreciate the work.

If @nateabele approves of this API change, I'll want to merge this in manually, tweaking lines 1002-1020 to closer match the ui-router coding style.

locals = toLocals[keep] = state.locals;
keep++;
state = toPath[keep];
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The block above should be something closer to:

var reloadState = options.reload === true ? fromPath[0] : findState(options.state);
 if (options.reload && !reloadState) {
          throw new Error("No such reload state '" + options.reload + "'");
        }
var skipTriggerReloadCheck = !!reloadState;
        while (state && state === fromPath[keep] && state !== reloadState) {
          locals = toLocals[keep] = state.locals;
          keep++;
          state = toPath[keep];
        }

Copy link
Contributor

Choose a reason for hiding this comment

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

@christopherthielen Do you want this fixed before we merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, wait looks like it was already fixed?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, they already made the changes I asked for

@oriweingart
Copy link
Contributor

Done, refactor the code according to your comment.

@christopherthielen christopherthielen added this to the 0.2.14 milestone Mar 10, 2015
@christopherthielen christopherthielen self-assigned this Mar 10, 2015
@nateabele
Copy link
Contributor

👍 Just squash the commits.

 - modiefied options.reload parameter in transitionTo method to accept the
   following:
   {reload : true} will reload all states
   {reload : 'foo.bar'} will reload foo.bar, and any children
   {reload : stateObj} will reload state found in stateObj, and any
   children.
 - added 'state' parameter(string|object) to $state.reload method,
   to be passed to $state.transitionTo method as the reload option.
 - test the $state.reload with state parameter
@maoryosef
Copy link
Author

Commits were squashed

nateabele added a commit that referenced this pull request Mar 30, 2015
feat($state): added 'state' to state reload method (feat no.1612)
@nateabele nateabele merged commit bf0f6de into angular-ui:master Mar 30, 2015
@lefterisk
Copy link

Good work people. @nateabele , someone should add this in the documentation, it took me half a day looking into workarounds for this problem only to find that its solved already, just not mentioned in the documentation. (This is a common problem apparently and people are using some scary workarounds)

@nateabele
Copy link
Contributor

@nateabele , someone should add this in the documentation

@lefterisk Go for it. 😄

lefterisk added a commit to lefterisk/ui-router that referenced this pull request Oct 30, 2015
This feature was added to the codebase by pull request angular-ui#1809 but was never add in the documentation.
@lefterisk lefterisk mentioned this pull request Oct 30, 2015
@lefterisk
Copy link

@nateabele created pull request #2347 for this :)

@nagyg-bvictor
Copy link

I'm not sure if I understand this right, but the reload string must be an absolute state name? I want to pass a relative state path like '.child'. I think it would be more straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants