-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add cancel button in ListPartNavigationAdmin.cshtml #18656
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
base: main
Are you sure you want to change the base?
Add cancel button in ListPartNavigationAdmin.cshtml #18656
Conversation
The added cancel button uses - href="javascript:history.back();" navigation back to the previous page.
|
Thank you for submitting your first pull request, awesome! 🚀 If you haven't already, please take a moment to review our contribution guide. This guide provides helpful information to ensure your contribution aligns with our standards. A core team member will review your pull request. If you like Orchard Core, please star our repo and join our community channels. |
@dotnet-policy-service agree |
Piedone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! When addressing review feedback, please adhere to the following:
- Please update your pull request according to feedback until it is approved by one of the core team members.
- Apply suggested changes directly so the reviewer doesn't have to eyeball the changes. These resolve themselves after applying them, and that's fine.
- Don't resolve other conversations so it's easier to track for the reviewer. Then, the reviewer will resolve them.
- Feel free to mark conversations that you addressed to keep track of them with an emoji or otherwise, just don't resolve them.
- Please keep conversations happening in line comments in those convos, otherwise, communication will be a mess. If you have trouble finding them, see this video.
- When you're done addressing all feedback of a review, click "Re-request review" in the top-right corner for each reviewer, so they know that you're done.
| } | ||
| @if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.ListContent, Model.Container)) | ||
| { | ||
| <a class="btn btn-secondary" role="button" href="javascript:history.back();">@T["Cancel"]</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not really canceling anything (unlike under content item editors); Simply "Back"? Also, I'd rather put in on the left side of this section, since it's navigation, not an action (an actual cancel would be like that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree! It is now changed in the latest commit: 9c0d5ce as per your suggestion.
| } | ||
| @if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.ListContent, Model.Container)) | ||
| { | ||
| <a class="btn btn-secondary" role="button" href="javascript:history.back();">@T["Cancel"]</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a returnUrl is set, that should be used instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we would have preferred to have used the built in returnUrl, but the problem is that in instances where we go two step further away from Content Items, it returns us and set us in an infinite loop due to how returnUrl works. For example:
Content Items -> Edit Blog (returnUrl = Content Items) -> Create Blog Post (returnUrl = Content Items + Edit Blog) -> Cancel goes back to Edit Blog (returnUrl = Content Items + Create Blog Post) -> Cancel goes back to Create Blog Post (returnUrl = Content Items + Edit Blog) ... and now we're stuck in a loop where clicking Cancel/back only adds then subtracts the returnUrl.
Hence we chose to solve it using: javascript.history.back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then that's what needs to be fixed, because going back in history is not universally correct.
… adhere to UX and per request.
The added cancel button uses - href="javascript:history.back();" navigation back to the previous page.
This PR solves the issue #6666