Media Picker 3 - It needs better backward compatibility and ease of use

[DanDiplo]

Umbraco version

v8

Description

The new Media Picker 3 introduced in Umbraco 8.14 and also present in v9 adds some great new features and has a much nicer UI. However, it also creates many issues which don't seem to have been considered.

The Problem

Firstly, it's not backward compatible with the previous Media Picker and there are no plans for migrations to be included. This creates a huge problem when upgrading existing sites as it requires editors to re-select every single piece of media and then republish the pages. For large sites, this is a massive undertaking.

Secondly, the property value converter no longer returns IPublishedContent but now returns a completely new type of MediaWithCrops.

public class MediaWithCrops    
{        
        public IPublishedContent MediaItem { get; set; }
        public ImageCropperValue LocalCrops { get; set; }   
 }

In one fell swoop all code that has been written that assumes Media is IPublishedContent falls over and all current documentation, blog posts and articles on rendering media have been made obsolete. As far as I can tell, none of the official docs have been updated to show how you are supposed to deal with it - they still say it returns IPublishedContent . I'm guessing that lots of packages etc. and existing code is going to require a lot of rewriting to deal with this.

If you install the default starter kit and swap the old media picker for Media Picker 3 you get exceptions galore. The site I tried to upgrade it to was strewn with exceptions where it expected IPublishedContent but got a different type or converters were throwing JSON parse errors all over the place - hardly a friendly experience.

This is further compounded by the fact that the old Media Picker is now hidden in Umbraco, meaning if you want to create a new media picker property you are forced down the road of using the new Media Picker 3. So now, even if you leave an upgraded site using the old picker, if you create new media properties these are now not compatible (so your rendering code now has to work with two different types, which is horrible).

Thirdly, adding local crops to a generic media picker makes little sense, since crops are only relevant to one particular type of media, namely bitmap-style images. It makes little sense to have a media picker that can be restricted to only picking, say, Word or PDF files, and yet have the cognitive overload of crops. Crops don't belong on media in general they are image specific.

My Suggestion

Why not simply update the UI of the existing Media Picker 2 so that it has all the nice features that Media Picker 3 offers but remove the local crops feature? This would have the advantage that it would allow seamless upgrades for people updating Umbraco and also continue to use IPublishedContent so that existing media rendering code doesn't need to be changed. I personally have little interest in using local crops, but do like the UX and UI of the new picker.

Then create a new Image Picker with Crops datatype that has this feature which is specific to images. After all, crops are specific to images, so why not make this explicit?

How can you help?

No response

[FransdeJong]

I have to agree on this one. I haven't played yet with media picker V3 yet but this looks like a oversight indeed.

I really love the interface and restrictions but this should indeed return IPublishedContent.

How will this work with existing crops? It'll have local crops and also the crops accessible with an alias?

I'm afraid we are stuck now since fixing this would be a breaking change?

[DanDiplo]

Yes, thinking about it, I can't see why this couldn't return IPublishedContent and then expose the extra crops as either another property or maybe an extension method? That way it would be backward compatible. Is it possible to override an inbuilt property value converter, I wonder?

[FransdeJong]

@DanDiplo
If we could override a inbuild PVC then we could make a "backwards compatibility package".
Although that should be the last resort.

[nul800sebastiaan]

Hey all, thanks for the feedback!

As noted in issue #10308, we don't have a good way to make the upgrade experience good for everyone.

We definitely considered if we could help with the upgrade from Media Picker v2 to v3 but we could not find a good way to do it:

First of all, and most importantly, the new model on the frontend is incompatible with MP2 (I am waiting on feedback from the HQ team as to why this model was chosen).

Even if we had made the model an IPublishedContent, we'd still need some kind of new extension method like GetLocalCropUrl, so your Razor templates would still need updating, something which the upgrader can't do.

As mentioned before, we have experience with trying to upgrade existing types and not only are we scared to kill people's sites due to the amount of data in it, we also know that there will be edge cases in the previously stored data that we can't predict, meaning your upgrade will fail. We want to make upgrades as painless as possible.

Most of all, we try to deliver value, and from your passionate feedback it seems like we did exactly that! 😅 Working on upgrading existing pickers had way too much uncertainty for us to even be able to predict when we would be able to get it done.

In addition to that, it also makes sense for a brand new datatype to be tested out first by people doing a fresh install of Umbraco, we might discover some interesting use cases and bugs we hadn't thought of. If we force all upgraders to use it from day one, this would create a lot more pressure on getting those fixed immediately. Gradual adoption gives upgraders the chance to wait for new releases before starting to use the "new and shiny".

We're of course happy to assist in getting a convertor package ready if anybody building something like that has questions.

We hope you realize that we don't make a decision like this easily, we have a lot of variables to consider. That said, of course we should have been better at explaining this up front when we released the new version. A good lesson for the next time! 👍

[FransdeJong]

Hi Sebastiaan,

Thank you for your answer.
I think you are focusing a little to much on why there is no upgrade path. The missing upgrade path is totally understandable in my opinion.

The main reason why we need a upgrade in the first place is the new model. This deviates from the "every content item is IPublishedContent" pattern we are using for so long now.

If the new picker uses IPublishedContent and a extensionmethod it would have been a painless transition and no migration was needed. Yes you need to add an extra extensionmethod if you want the extra functionality but it's the same with the "old way" of using imagecrops.

So the main questions are:

• why the new model
• is there any chance of rethinking the mediapicker V3 model or is this a done deal?
• will mediapicker V2 be deprecated and when?
• can we expect more editors moving away from IPublishedContent in the future?

Frans

[DanDiplo]

Thanks for the feedback, @nul800sebastiaan

Could you at least, then, consider un-deprecating Media Picker 2? Currently there is no way I can upgrade any sites to Umbraco 8.14 because of the lack of backward compatibility. As Franse says, the main issue is the breaking-change of introducing a different model for the picker that doesn't inherit from IPublishedContent.

Currently if I upgrade a site and then subsequently want to add a new image property I am forced to use Media Picker 3. This then means that half my site is returning media as IPublishedContent and half as MediaWithCrops. That's just not something I can handle without rewriting a lot of code. It's an absolute deal breaker, and means where I work we will have around 50 sites unable to be upgraded. I can handle writing code to migrate if I have to, but I can't handle rewriting all the code I have that deals with Media.

Are people on Cloud aware of this? Do they know what will happen if they upgrade to 8,14 and then want to add a new picker?

The best solution would be to change the return type to follow the IPublishedContent pattern that has been the default for all this time. But if you can't, then at least make Media Picker 2 pickable again so we can continue with that. Otherwise I think you are going to have major issues on hand once a lot of people realise this. Better to bite the bullet now, IMO.

[nul800sebastiaan]

why the new model

I'm waiting for some HQ input on that, but I think it's because the IPublishedContent is the Media item which doesn’t have the local crops. The local crops are only stored on the property value itself, so there would be no way to retrieve the extra data if the return type was IPublishedContent.

is there any chance of rethinking the mediapicker V3 model or is this a done deal?

I doubt it, waiting for HQ response.

will mediapicker V2 be deprecated and when?

It is in 8.14. Fresh installs no longer see MediaPicker v2.

can we expect more editors moving away from IPublishedContent in the future?

I don't know 😄 We'll need to figure that out when we encounter a new datatype to install.

[nul800sebastiaan]

Could you at least, then, consider un-deprecating Media Picker 2? Currently there is no way I can upgrade any sites to Umbraco 8.14 because of the lack of backward compatibility.

Absolutely! This is a config option: https://our.umbraco.com/documentation/reference/config/umbracosettings/#obsolete-data-types

And any existing MP2 datatype will continue to be MP2.

[FransdeJong]

Is Mediapicker v2 still present in V9? Otherwise this change would block the migration from v8 to v9. It's impossible to resave all images as media cropper v3 before migrating especially on larger sites.
Mediapicker 2 being deprecated insinuates it being removed in the next major update?

[nul800sebastiaan]

It's still in v9, in the same state: obsolete. Same config option as above allows you to create new MP2 editors.

[mattbrailsford]

Couldn’t MediaWithCrops implement IPublishedContent and just have it proxy to the picked media item. Then if you want the local crops functionality you can cast it to MediaWithCrops?

[FransdeJong]

That sounds like a awesome solution serving both backwards compatibility and also new functionality.

@nul800sebastiaan
Maybe this issue should be reopened?

[abjerner]

Couldn’t MediaWithCrops implement IPublishedContent and just have it proxy to the picked media item. Then if you want the local crops functionality you can cast it to MediaWithCrops?

Wouldn't that break ModelsBuilder? As the ModelsBuilder models don't inherit from MediaWithCrops.

---

I haven't really looked at the code, but couldn't we do something like:

1. Un-obsolete Media Picker 2
2. Rename Media Picker 3 to Image Picker
3. Possibly create a directive that can be used for both editors (shared, but configurable logic)

IMO the new media picker doesn't really make sense for anything else than images, so calling it Image Picker would make the purpose more clear. And then Media Picker 2 could be un-obsolete as I believe it still has plenty of valid use cases.

---

Possibly going a bit off topic, but what I really would have liked to see is a custom view / data type extending the block list.

Today, inserting an image is so much more than just the image (and crops). It could be that the image needs an in-context description to fulfill accessibility requirements (alt-text property directly on the image may not be enough). Could be that editors need to be able to specify whether an image is decorative or not. Or something else that is unrelated to accessibility. The image and the crops could be two different properties, instead of being part of the same property.

Having a block list consisting of image items rather than just a list of images would give an extra step (or two) or flexibility and control to the editors. And adding a custom image list block list view would make than experience even more awesome.

As we need something like the block list before the block list was ready, we ended up building our own Elements package, which lets you create a list of IPublishedElement. A bit similar to the block list, but without the separation of content and setting.

I've attached two GIFs to illustrate how it works. First GIF is the standard list view of the package; you get a dull icon as preview, and if you click an item, an overlay is opened so you can edit the item. You can only add a single image at a time, and the properties really don't know about each other.

The second GIF shows the use of a specialized view tailored for an image list (our ImagePicker and Elements packages working in tandem). You get a nice preview of each item/image, the title property is automatically filled with the name of the image, and you can add multiple images to the list at the same time.

Our LinkPicker works the same way be providing a specialized view for creating a link list, as links may also require some extra properties beyond what the standard link picker overlay and multi URL list editor provides.

We build this because we needed the flexibility, but I'd very much like to see something like this in Umbraco. This would also be something that is in addition to using a media directly, instead of replacing it.

[mattbrailsford]

I do like the idea of the block list based editor. Don’t think we could quite get as nice a UX, but I’d at least wish it used IPublishedContent/Element to keep the power it offers.

—

Couple of things I’m pondering, should changes like this that are non optional be considered breaking changes and so shouldn’t be in a minor release? And secondly, should significant changes to prop editors be discussed in RFC’s maybe being deployed via a feature flag to allow people to try it if they want to and give this really important feedback? I’m not sure I like the approach of pushing it out and using pushback to shape it once it’s out. We don’t want another UserControlGrapper.

An interesting read on feature flags used by GitHub https://github.blog/2021-04-27-ship-code-faster-safer-feature-flags/

[abjerner]

Honestly I've been bad at keeping up with RFCs and release candidates, but yes, and RFC could probably be the way to go, as it would probably be too late to address something like this in a RC.

IIRC, Umbraco has had breaking changes in major minors before, so I guess this would be in line with that. But I assume MC2 will still be around for the lifetime of Umbraco 8, so in that regard, it may not be a breaking change.

[FransdeJong]

It would be great if there is a documented way to migrate from V2 to V3. Especially for existing content.
I don't mind having new types I gues...

[DanDiplo]

Couldn’t MediaWithCrops implement IPublishedContent and just have it proxy to the picked media item. Then if you want the local crops functionality you can cast it to MediaWithCrops?

This is exactly what I was suggesting. It maintains backward compatibility with how previous media pickers are consumed, which will save a lot of confusion and extra work in the long run. Imagine, for instance, how much documentation, blog posts, articles, forum articles, tutorials and videos are out there that expect media to be IPublishedContent, as it's always been? How confusing will this be for new Umbracians when none of those work any more? How annoying will it be for people to have to rewrite every single line of code dealing with images and media to take into account these new types? It just seems like a completely unnecessary breaking change.

I also suspect that if the return type was kept as IPublishedContent then the JSON would remain backward compatible with Media Picker 2. This means people could easily swap in the new media picker without losing all their existing media - far more friendly!

I realise that changing the type now will cause some issues, but not changing the type will cause even more issues. I really feel a big mistake has been made and there's now an opportunity to fix it. Someone remarked to me today that Umbraco HQ like to "double down" on mistakes - and there's some truth in this, I'm afraid. We're all human and make mistakes - and that's fine - so let's find a way to solve this and not just close the issue and pretend it's not happened.

[nul800sebastiaan]

Thanks all, again, for your extensive feedback. I'm afraid we're now starting to talk about a lot of different things at the same time, I will not be able to respond to the feature requests at this time, they're great, and should be discussed and we'd love to give some feedback, but please add them to a new discussion: https://github.com/umbraco/Umbraco-CMS/discussions/new?category=features-and-ideas

As for obsoleting the v2 media picker, we hear your feedback loud and clear. It was moving too much of your cheese too soon!
With that said, we will "un-obsolete" the Media Picker v2 so people can keep using it and create new ones without having to change a config option.

We love RFCs and they're very important, but they also require a great deal of time to manage. That said, from the feedback it's clear that we should re-think how we introduce bigger nee features like this. We've often talked about feature flags, thank you for the suggestion again here! 👍 I love feature flags personally and we're going to have a deeper look into that at HQ over the next few weeks.
We understand that it's hard to keep up with all the new things happening in Umbraco releases all the time and it's good to talk about new features earlier and more often. This is not always possible in RFCs but maybe a combination of "mini-RFCs" and feature flags would work very nicely.

There is good reasons for having the model as it is and I will steal some words from my colleague to describe those.

On why there's both an IPublishedContent an MediaWithCrops property on the model:

Because the IPublishedContent is the picked Media item which doesn’t have the local crops. The local crops are only stored on the property value itself, so there would be no way to retrieve the extra data if the return type was IPublishedContent

On the suggestion to add (for example) a GetLocalCrops extension method on IPublishedContent:

Adding a property editor specific extension method to IPublishedElement/Content would make it hard to discover how to get the value back from the property editor, since you need to know about the GetLocalCrops method that only works when you have a MediaPicker3 property on your content (I know that’s how GetCropUrl works but at least we ship the Image media type with core). I also don’t think an extension method would work when picking multiple media items, since it has to be called on the content containing the property. Last but not least 🙂 if I wanted to use some of the other values stored on the property, I would then need to get the raw value myself and then parse it as json to the correct model etc. which is not a great experience.

We appreciate that existing blog/form/video content is no longer relevant for the new media picker. We had a brainstorm about trying to name it something else but it seems like "this picks items from the media section" is too perfectly described in "Media Picker" so we couldn't come up with something better, suggestions welcome of course!

FYI: MediaPicker v2 will be around for the lifetime of v8 AND v9, quite likely it will still be in v10 as well, we'll have to determine that at a later point though.

We're excited about Media Picker v3 and we're exploring more updates to make it even more useful in the future!

[DanDiplo]

Hi @nul800sebastiaan Thanks for taking the time to produce such a thorough response and for giving us clarity. If it's staying the way it is, so be it - at least I know and can start to work around this now. Good to know Media Picker 2 will be "un-obsoleted" as I'm not sure it would be obvious to people how to do this.

One thing I would ask: Is there any chance any of the UI/UX improvements made to v3 of the picker could be back-ported to v2? I think that would mitigate a lot of concerns, as people would benefit from the much-improved UI whilst still maintaining backward compatibility. Would you be open to that?

[nul800sebastiaan]

@DanDiplo Sorry for the late reply, this CodeGarden thing you might have heard about happened.

In reality I don't think we want to spend a lot of time making the old picker much like the new picker, we're moving on - if you're using the old picker it works as expected and has been just fine for all these year. The new picker is something to explore in new projects.

However, secretly I've been trying to see if we can convert data progressively without breaking things, and I think that might be coming to a patch release soon. It will require people to update their templates and I know you're not a fan of that, but in that case you can write your own property value converter to give you less data (as in: you can not get the data for the crops). So as long as you don't configure local crops, a custom PVC would work. You would never be able to add local crops in the future either though. Anyway, let's see if my work on that will be approved, but it should pave the way for people switching from MP2 to MP3 with the caveat that they'd need to update their templates. 🤞

[DanDiplo]

@nul800sebastiaan Yeah, I've heard of that Codegarden thing. Apparently quite popular, isn't it? :)

OK, fair enough. I might just weight until Umbraco 9 then to use the new MP3. I've got a lot of existing projects with a lot of media and rewriting all the image rendering code would be a pain. Just feels a shame there wasn't an easy way forward.

One thing I have noticed is that there have been a lot of posts on Our forum from people not understanding that the new picker no longer returns IPublishedContent so you might need some better publicity / documentation on this, as it's catching a lot of people out.

Anyway, thanks for the update and take care 👍

[nul800sebastiaan]

Update: it's been merged - In the soon upcoming release of 8.15 you will be able to progressively save MP2 as MP3 with the caveat that you will need to update your template code: #10517

As noted there, if you wanted to you can make a custom property value converter to just return a list of IPublishedContent again, which means it will be difficult to use the additional local crops feature, though I'm sure that could also be fixed with an added extension method if you really wanted to.

[nul800sebastiaan]

We totally understand that there will be confusion for a while around code examples, and that hurts for a bit but people will adapt eventually. 🙂

[ronaldbarendse]

Making the data structure backwards compatible with MP2 (as done in #10517) will remove the need to manually migrate existing data (and all issues ensuring you don't end up with the old data format again, like restoring older versions or content from other - not migrated - environments).

Changing editors is something you'd do on a development environment/locally and it's very common this also changes the returned model on the front-end (e.g. changing a Textarea to Richtext editor also changes the model from string to IHtmlString). You currently have to update your front-end code to correctly handle the new MediaWithCrops model, but both the data type change and updated views can be deployed together (using Deploy or uSync for the metadata).

We do recognize working with the new model and local crops (together with the media crops) can be improved, so I've created a PR to also make this a better experience (and require no - or at least less - changes on the front-end): #10529.

[nul800sebastiaan]

As an update: I was wrong! 😅 Somehow Ronald seems to have made the impossible possible.. 8.15 RC is out now allowing you to seamlessly switch from MediaPicker 2 to MediaPicker 3. The returned results on the frontend will remain IPublishedContent and you can get the local crops by using "good old" GetCropUrl, which will look for a local crop with the provided alias first and then for a crop on the media item, if a local crop doesn't exist.

Thanks again for all your feedback! We fully acknowledge that it would have been much better to have this conversation before releasing anything and that's our goal from now on when we introduce larger features 👍

Hopefully this should cover all cases above and we'll see a lot more MP3 uses in the near future! 😁

A friendly request for people already on 8.14: this should "just" work but we'd LOVE it if you could do a test upgrade to see if we broke anything we didn't expect please 🙏