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

[RFR] Return to list view when deleting a record #642

Merged
merged 8 commits into from
Sep 14, 2015

Conversation

jpetitcolas
Copy link
Contributor

Needs to implement and fix tests.

Closes #641.

body = JSON.stringify(body);
}

this.back();
Copy link
Member

Choose a reason for hiding this comment

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

Why do you go back in case of error? Doesn't make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of #538

But true, there will be another issue here: what about deleting a record from a reference_many list?

A solution may be to add a "with_redirect" to true by default, and open the confirmation box in a pop-up. Reference many delete button would not redirect in this case. What do you think of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To remove

@fzaninotto
Copy link
Member

One thing that concerns me is that if you're coming from a filtered list you'll lose the filter.

I think you should be smarter and keep the back() when coming from a list, and going to the list otherwise.

@jpetitcolas
Copy link
Contributor Author

After an IRL discussion, the most easy way to achieve it is to always redirect to the previous page.

We only redirect to the list view if previous page has the same entity and id than the deleted object. In this case, previous view doesn't exist anymore, whatever it is (show or edition view).


this.previousStateParametersDeferred = $q.defer();
$scope.$on('$stateChangeSuccess', (event, to, toParams, from, fromParams) => {
console.log(fromParams);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Console log spotted

@jpetitcolas
Copy link
Contributor Author

Switching to RFR.

@jpetitcolas jpetitcolas changed the title [WIP] Return to list view when deleting a record [RFR] Return to list view when deleting a record Sep 10, 2015
@jpetitcolas
Copy link
Contributor Author

Tests should now be green: I removed sinon dependency which caused some troubles with Webpack (it was not used before this PR).


this.previousStateParametersDeferred = $q.defer();
$scope.$on('$stateChangeSuccess', (event, to, toParams, from, fromParams) => {
this.previousStateParametersDeferred.resolve(fromParams);
Copy link
Member

Choose a reason for hiding this comment

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

How about if I click a link in the navbar rather than deleting?

Copy link
Member

Choose a reason for hiding this comment

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

You need to bind once, or stop the listener after deletion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be unbound at controller destruction (as bound with $on).

@fzaninotto
Copy link
Member

unit tests fail

@jpetitcolas
Copy link
Contributor Author

Green! \o/

fzaninotto added a commit that referenced this pull request Sep 14, 2015
[RFR] Return to list view when deleting a record
@fzaninotto fzaninotto merged commit e183397 into master Sep 14, 2015
@fzaninotto fzaninotto deleted the show_list_redirect branch September 14, 2015 14:08
@fzaninotto
Copy link
Member

Thanks!

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.

404 after successful delete
2 participants