search

callumbwhyte / meganav

A flexible, draggable link picker for constructing site navigation menus in Umbraco
MIT License
35 stars 34 forks source link

Feature/udi support #26

Closed kows closed 5 years ago

kows commented 6 years ago

25

To get things starting, editor is still working on local environment but did not deploy it to cloud environment to test transfers.

Not sure when upgrading old values will still be working.

kows commented 6 years ago

A little bump for this :)

mzajkowski commented 6 years ago

Hmm shouldn't we think here about backwards compatibility or just support Umbraco from specific version in the future releases?

alindgren commented 6 years ago

While backward compatibility should always be a consideration for packages, in this case UDI support might be worth it. Besides, we are on the eve of the 7.11 release and this still works back to 7.6.2. If you are working on open PRs for a new release -- maybe do the other PRs first and release a version without this PR so pre-7.6.2 sites can get them, and then move forward. Just my 2 cents -- looking to use this on a new project soon :)

kows commented 6 years ago

In the spirit of the hackathon, any idea when you could handle my PR's? :)

mzajkowski commented 6 years ago

@kows thanks for pushing us! I promise to also chase @callumbwhyte tomorrow and maybe we'll be able to properly test it and merge it.

Thanks for your effort!

ronaldbarendse commented 6 years ago

Just some notes: mapping from an UDI to IPublishedContent is slower than using IDs (altough they added some additional caching in Umbraco 7.10.0): http://issues.umbraco.org/issue/U4-10756.

As this feature is primarly suggested to add Umbraco Cloud/Deploy support, I would suggest using a deploy connector instead. See my PR for this: https://github.com/umbraco/Umbraco.Deploy.Contrib/pull/25.

And if this get merged, I would recomment dropping the ID property completely and release it as version 2 (because of this breaking change). If you take a look at the models of https://github.com/rasmusjp/umbraco-multi-url-picker, you can see how messy keeping the ID alongside the UDI can become!

BTW The work already done in this PR will probably be required for Umbraco 8 if they go the all-UDI route! Maybe this can already be tested on the alpha versions?