OrchardCore
OrchardCore copied to clipboard
OrchardCMS
13715: Added edit button to the ContentPickerField Edit view
Fixes #13715
Added an edit/view button to the ContentPickerField.Edit view which allows to open the selected content item in a separate tab.
Example:
Dont know if this is enough or if it's wanted to control the visibility of the button via the field settings?
First time working with vue so feedback is more than welcome, especially on the part with replacing the the content item id into the edit url.
Animated GIF it would be helpful to show the final result
Added an example image with multiple items
Added an example image with multiple items
So, you didn't open a new window, right?
Added edit button to the ContentPickerField Edit view
I didn't see the edit button on the screenshot!!
Just an idea on the fly ;)
We needed this in a project where we provided a linkable displayText targeting the editor in place of the additional button, then we had to manage to return back to the parent on save/publish/cancel.
To return back to the parent we had to take into account the use case where the CPField is inside a bagPart, as I remember we had to convert the full uri of the "referrer" header to an absolute url.
@jtkech that is better to me that adding Edit button. You can check if the user can edit, render the display text as a link to the edit page using returnurl as current url. If the user does not have edit, but has view access, show a link to the display page otherwise just display text.
@jtkech @MikeAlhayek I can take a look into going that route, but Im off for some weeks. I will work on this afterwards again. Thanks for your feedbacks
Is the text displayed in the list of items configurable?
If it's the same as the current field then it's ok. Otherwise maybe we should use a custom liquid template
Is the text displayed in the list of items configurable?
If it's the same as the current field then it's ok. Otherwise maybe we should use a custom liquid template
I didn't change anything with how the DisplayText is rendered of an item. My type just didn't have a TitlePart set, so it's the default ToString of an ContentItem.
Im back now and I hope I can continue working on this in the next days
Am I right that the View permission is actually implicitly inherited by every role and thus the check if an item is viewable is unnecessary? Do you know that @jtkech or @MikeAlhayek ? Also @jtkech do you think the returnUrl is something we want to have for this one?
@dannyd89 the view permission is inherited by default. However, you can change that behavior by removing "View all contents" from the Anonymous role. So the view check is necessary.
The returnUrl will be important to make the user experience better. If the user is sent to the edit view and they published, you want them to be returned to the content item you were on before the navigation away.
IMO, we should have a way to add/edit a related content item using a modal "on the same screen" as it'll have better user experiences. This idea is logged part of issue #11648.
I think Im finished with the implementation of everything which was requested.
Here you can see an example of a type with a Bag and current logged in user can only view the first 2 items and the other item is restricted to him, so it's not even viewable.
The Display urls dont have any returnUrl attached, because you cant do anything on those pages, so I didn't add any returnUrl parameter to it.
With a user which has the permission to edit those items, the UI does look like this:
As you can see, the returnUrl is added as a query param and if the user publishes or cancels on the Edit page, he will return to the item with the ContentPickerField he clicked on. The returnUrl contains everything currently in the url string, so even if there is another returnUrl param, it will be captured, so you can click on one item in the ContentPickerField, return to that and if you publish again, you will be returned to whereever you were before.
I hope this contains everything which was requested.
Review is very much appreciated @MikeAlhayek @jtkech
This pull request has merge conflicts. Please resolve those before requesting a review.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request has merge conflicts. Please resolve those before requesting a review.
I sincerely apologize for us taking so much time here. I checked out the PR, going over old ones, and this would be quite useful.
@dannyd89 could you please fix the merge conflict, and @MikeAlhayek please get back to the review?
I sincerely apologize for us taking so much time here. I checked out the PR, going over old ones, and this would be quite useful.
@dannyd89 could you please fix the merge conflict, and @MikeAlhayek please get back to the review?
Resolved all conflicts. Would be great if this PR is merged soon
Maybe I'm late and it is already fixed but why do we get a "view icon" when the action is "edit"? Should we not have an edit icon then? Because, that's what it is if it opens up the edit page of that content item.
Maybe I'm late and it is already fixed but why do we get a "view icon" when the action is "edit"? Should we not have an edit icon then? Because, that's what it is if it opens up the edit page of that content item.
It's not a button anymore, you can see how it looks with the latest screenshots I posted in the comment above
@dannyd89 when the user click the edit or display link, we should open the new link in a new tab by default. If you are in the middle of editing a form and click on a link, you do not want to lose the data you had in the current form.
@dannyd89 when the user click the edit or display link, we should open the new link in a new tab by default. If you are in the middle of editing a form and click on a link, you do not want to lose the data you had in the current form.
But then the returnUrl parameter doesnt make much sense, does it? I mean I changed it to this way because it was requested, should I change it back?
Hum! The returnUrl maybe helpful if we are just viewing a content, then we click on a user, save changes related to the user and want to get back to the content edit page. I don't know that is a helpful in terms of on UI.
If you are creating a new content, and filled the form half way, then in the user picker you clicked on a user. Now you navigated away, even if you save the user and return back using the returnUrl, you already lost the data you had in the form. And if I end up not saving the user, I am lost and will have to either go back using browser or add new content item.
I think it is better to open another tab if the user click on a user. This way they don't lose any work they have done on current form.
Hum! The returnUrl maybe helpful if we are just viewing a content, then we click on a user, save changes related to the user and want to get back to the content edit page. I don't know that is a helpful in terms of on UI.
If you are creating a new content, and filled the form half way, then in the user picker you clicked on a user. Now you navigated away, even if you save the user and return back using the returnUrl, you already lost the data you had in the form. And if I end up not saving the user, I am lost and will have to either go back using browser or add new content item.
I think it is better to open another tab if the user click on a user. This way they don't lose any work they have done on current form.
Understood, I changed it so each click will open in a new tab
This pull request has merge conflicts. Please resolve those before requesting a review.
@MikeAlhayek, can't this be now merged? We also need to resolve the merge conflict unless @dannyd89 you can do that (you just need to rebuild the client-side assets, see https://docs.orchardcore.net/en/latest/docs/guides/gulp-pipeline/).
Ah OK, just because I was wondering why this was left unmerged after your approval. If you're OK with it them I'm OK as well, I don't think much else is needed here. I merged, could you please do a last check and merge if OK?