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

Umbraco Forms contains two namespaces for MVC #488

Closed
bjarnef opened this issue Feb 18, 2021 · 6 comments
Closed

Umbraco Forms contains two namespaces for MVC #488

bjarnef opened this issue Feb 18, 2021 · 6 comments

Comments

@bjarnef
Copy link

bjarnef commented Feb 18, 2021

In Umbraco Forms v8.6.0 I noticed UmbracoForms.Core contains two namespaces for "MVC".

Umbraco.Forms.MVC and Umbraco.Forms.Mvc

It seems a bit strange that these doesn't share namespace?

image

It seems Umbraco.Forms.MVC only contains the Umbraco.Forms.MVC.Extensions.JQueryUIDatePickerHelper class.
Is there a reason this doesn't exists in Umbraco.Forms.Mvc namespace?

Furthermore I notcied Umbraco.Forms.Core.Configuration.GetSetting() is marked as deprecated and we should use FacadeConfiguration. How do we access this? I doesn't seem it is documented and so far I have only found Umbraco.Forms.Core.IFacadeConfiguration.

image

@bjarnef
Copy link
Author

bjarnef commented Feb 18, 2021

The current way:

var test = Umbraco.Forms.Core.Configuration.GetSetting("myCustomSetting");

It seems the following works:

var configuration = Umbraco.Web.Composing.Current.Factory.GetInstance<Umbraco.Forms.Core.IFacadeConfiguration>();
var test = configuration.GetSetting("myCustomSetting");

@bjarnef
Copy link
Author

bjarnef commented Feb 18, 2021

Okay, regarding the IFacadeConfiguration it seems we can inject this in the controller in newer versions of Umbraco Forms. Not sure about the approach in views though?
https://github.com/shaishavkarnani/RecaptchaV3/pull/2/files

Maybe the obsolete message should say "Use IFacadeConfiguration" instead?

@nul800sebastiaan
Copy link
Member

Thanks for reporting @bjarnef - our lovely legacy codebase is sometimes a bit nonsensical. We'll have a look at it.

@nul800sebastiaan
Copy link
Member

Not sure if you're still struggling with this or found the workaround? For now "just" use the wrong namespace I guess?

@nul800sebastiaan nul800sebastiaan added project/api state/sprint-idea This issue doesn't fit in the next few sprints but we're reviewing it regularly for a later sprint type/bug labels Feb 23, 2021
@bjarnef
Copy link
Author

bjarnef commented Feb 23, 2021

@nul800sebastiaan the easiest would probably be to update for namespace for JQueryUIDatePickerHelper to be inside Umbraco.Forms.Mvc, but maybe I breaking change if this helper is something used in form themes.

Regarding IFacadeConfiguration I found that I could inject this in the constructor of the custom field type.
In the view I guess we need to use the following:

var configuration = Umbraco.Web.Composing.Current.Factory.GetInstance<Umbraco.Forms.Core.IFacadeConfiguration>();
var test = configuration.GetSetting("myCustomSetting");

Would be great with some updated documentation regarding the new way to get a custom field setting.

@warrenbuckley
Copy link

warrenbuckley commented Mar 2, 2021

Namespace fixed in upcoming 8.6.2 8.7.0 release with this PR

@umbrabot umbrabot removed the state/sprint-idea This issue doesn't fit in the next few sprints but we're reviewing it regularly for a later sprint label Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants