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

Media Picker 3 documentation #3106

Merged
merged 26 commits into from
May 21, 2021
Merged

Media Picker 3 documentation #3106

merged 26 commits into from
May 21, 2021

Conversation

nielslyngsoe
Copy link
Member

@nielslyngsoe nielslyngsoe commented Apr 7, 2021

Documentation for the new Media Picker. Lets try to ensure that its not misunderstood what is Media Picker (v2, to become the old) and what is Media Picker 3 (the new one).

Notice for v8, both version will be available, In v9 I hope we can remove Media Picker v.2.

nielslyngsoe and others added 4 commits April 9, 2021 09:13
…Editors/Media-Picker-3/index.md

Co-authored-by: Mads Rasmussen <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: Mads Rasmussen <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: Mads Rasmussen <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: Mads Rasmussen <[email protected]>
Copy link
Contributor

@sofietoft sofietoft left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @nielslyngsoe !

I've made some suggestions here and there, and added some comments as well.

I think we need to make it a little clearer that there are now 2 different Media Pickers. Perhaps on the "old version" we link to the new, and somehow make it clearer that we recommend using the new one? What do you think?

nielslyngsoe and others added 12 commits April 13, 2021 13:53
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
…Editors/Media-Picker-3/index.md

Co-authored-by: sofietoft <[email protected]>
@sofietoft
Copy link
Contributor

Thanks for applying the suggestions to the article @nielslyngsoe !

About the different snippets in the bottom of the article, about how to fetch the global and local crops... Would it make sense to move that up above the MVC samples? 🤔
Also, does it make sense to have 7 different samples on how to fetch values from the property? Seems to me like it could be a bit confusing trying to figure out which one to use... What do you think?

@filipbech
Copy link

Need to update so it says 8.14 not 8.13

@nielslyngsoe
Copy link
Member Author

@sofietoft this manner was inspired by the current Media Picker documentation, I think we need to ask a C# developer on whether it's necessary or useful. if not we should correct both.

update version
Copy link
Contributor

@sofietoft sofietoft left a comment

Choose a reason for hiding this comment

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

I've given this a second review. Added some suggestions.

Also, I still think it's a lot with all those different MVC samples... 🤔 I'll try cleaning them up, once this has been merged in.

Regarding the crops.. I think it would be great if there was some more explanation about the two different type, local and global. Right now, it's only mentioned in the samples at the very bottom, and not really explained.
Would it make sense to talk a bit about the local and global crops somewhere further up in the article? There's already a little section about "Image crops" in the article. Perhaps it could be added there?

Does the addition to this article relate at all to this: https://our.umbraco.com/documentation/Getting-Started/Backoffice/Property-Editors/Built-in-Property-Editors/Image-Cropper/ ? 🤔 Should that article also be updated, or?

Also, I think it would be worth linking to the new Media Picker 3 article from the original Media Picker article, with a short note about how we recommend using the new one!

@nielslyngsoe
Copy link
Member Author

@sofietoft good point about mentioning Media Picker 3 in Image Cropper.

I also added the notice to the current Media Picker.

We would need some C# person to make sense of the MVC samples, I don't know if there is an audience for all of it and whether it's understandable for C# developer. Its a general problem in many of our property editors documentation.

Copy link
Contributor

@sofietoft sofietoft left a comment

Choose a reason for hiding this comment

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

Looks great @nielslyngsoe !

Thanks for adding the suggestions.
I'll make sure we review all the property editor articles at some point, so we can clean up all of these many different samples, that we list for each of them.

I'll make sure this is merged Thursday morning, so it'll be ready for the 8.14 RC.

@dampee
Copy link
Contributor

dampee commented May 19, 2021

I looked at a few C# samples. And they make some sense, but could be improved in the future.

@nielslyngsoe
Copy link
Member Author

@dampee thanks, would be nice to improve them... Currently, they require quite a high skill set to understand why and which one to go with, which is bad for newcomers. So if you or anyone else has ideas on how to communicate this so it becomes easier to learn Umbraco, would just be awesome :-)

I´m not super skilled with C#, so it would be nice with someone show knows what is right to have a look at this. :-)

@dampee
Copy link
Contributor

dampee commented May 19, 2021

Launching just an idea.

I don't know if it fits in our agenda of the docs workshop upcoming friday (11am - 1pm). The idea is that we are working on the docs at that time. I'll join the meeting from 11am till 12 as I have another meeting at 12:00.

Maybe we can work on it together for 20 minutes?

@nzdev
Copy link

nzdev commented May 21, 2021

Does this support using a different image per crop? Sometimes you need to use a different image if the original does not look good when cropped for some crops.

@nielslyngsoe
Copy link
Member Author

Hi @nzdev
I see the case but it's not a feature of this. From a UX perspective that would be rather complex to establish in a way where it's comfortable and easy for content editors to set up and navigate.
I think the best option, for now, would be to have additional upload fields for example the Image Media Type? Maybe create a new Multi-File Image Media Type, of which you have several Image Croppers (like one for wide-crop tall-crop, etc).

If the crops have to be local, I would recommend having several image pickers? And If you are dealing with a scenario where you need to pick multiple images, it might be an idea to make a Block List of an Element Type that holds those Media Pickers.

I hope that can inspire you to find a good solution for your case.

Interesting thought and it really got my mind started on thinking about different UI solutions.

How do you imagine it working. Like how would you set a different image for one or multiple crops? And how does another Content Editor spot that there is a different image for some crops?

@nzdev
Copy link

nzdev commented May 21, 2021

Hi @nielslyngsoe thanks for the reply. I would imagine you pick an image, then adjust the crops, see that for a certain local crop the image does not look good when cropped. Then on the local crop you can choose to select another image. Perhaps with a replace button that then let's you browse for another image. Then to revert have a reset button. Both the single and multiple images scenarios would be a slow workflow by the block list, storing the udi of a media per crop when overridden would be a great experience. I imagine the preview would need some sort of badge to show if any of the crops use a different image without trying to re crop. Overriding an image would be optional per crop.

@sofietoft sofietoft merged commit 4fcd0c1 into main May 21, 2021
@nul800sebastiaan nul800sebastiaan deleted the v813/media-picker-3 branch August 12, 2021 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants