OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.4k stars 2.39k forks source link

Each Custom User settings should have its own form #12453

Open MikeAlhayek opened 2 years ago

MikeAlhayek commented 2 years ago

Is your feature request related to a problem? Please describe.

Currently, when using CustomUserSettings, the custom settings is rendered in a new tab. However, all the tabs are part of the same html form. The problem with this setup is that a user must provide data in all the required fields "in all the tabs before saving". This make for a poor user experience.

In other words, if you have a user-custom-settings that captures user phone number and addresses. where at least one phone number is required, changing the content tab will not work because the user must update the phone number field in the custom tab.

Describe the solution you'd like

Each custom settings, should have it's own form so when the save button is clicked, only the data in the current page is validated and updated.

hishamco commented 2 years ago

I expect the form should be validated against the content fields validation

hishamco commented 2 years ago

I expect the form should be validated against the content fields validation

MikeAlhayek commented 2 years ago

Well yeah. that does not change. but instead of validating and updated data from all tabs, we should only validate/update of the current tab.

hishamco commented 2 years ago

Totally agree

ns8482e commented 2 years ago

I guess Default is single form that covers most, However I believe using placement and overriding templates you can achieve what you need

MikeAlhayek commented 2 years ago

@ns8482e this makes of a bad user experience. If we were to keep a single form, then maybe we should change the default look from tabs to card so everything is virtually on the same page and the user is not hunting for errors in multiple tabs.

ns8482e commented 2 years ago

@MikeAlhayek I am not denying that issue exists, I am saying is default behavior of tab placement uses same form as everywhere else in orchard and consistent.

if separate form is required then the use of GroupId is recommended

ns8482e commented 2 years ago

@MikeAlhayek I have not tested but I guess using placement you can set group as Content@profile Even setting card/tab on group should work too Content@profile#tab1

MikeAlhayek commented 2 years ago

Thank you Niraj! placement was used to override the default behavior. However, I think the default behavior makes for a poor user experience. I think the default should be changed to use "Card" instead on "Tab" by default so that the user can see all custom settings in cards so there is no need to fish around the tabs for errors.

This is what I added in my placement.json file to change the default placement.

{
  "CustomUserSettings": [
    {
      "differentiator": "CustomUserSettings-MyCustomUserSettings",
      "place": "Content:15%My Custom Settings"
    }
  ]
}
ns8482e commented 2 years ago

I see a need for Nav placement something like Content@setting(Custom Setting):10 that will place content on setting group on content zone at 10, but always render Navs with link to group route with title Custom Setting

sebastienros commented 2 years ago

I'd like to see how it looks like with cards then. And this way we could compare it with another option which is to highlight tabs which would contain validation errors.

MikeAlhayek commented 2 years ago

@sebastienros here are some screenshots.

Screenshot after the PR

image

Screenshot before

image

hishamco commented 2 years ago

I like the tabs in this case from UI/UX perspective

MikeAlhayek commented 2 years ago

@hishamco can you please explain why you think tab are better from UX experience? I suggest that you actually try it by adding 2 custom content type and make at least one field required on each custom content type. Then try to update the user by leaving both required fields empty (from both tabs) and click update.

ns8482e commented 2 years ago

@MikeAlhayek I also prefer tabs to organize content without vertical scroll

I also like all edit at once

MikeAlhayek commented 2 years ago

@ns8482e that is UI preference which is a different subject than UX. IMO, user experience should define the user interface. So when UI produces a poor user experience, we should consider adjusting the UI to provide best UX possible by default.

hishamco commented 2 years ago

As @ns8482e mentioned above tabs are great to organize content and you can see tabs headers at one glance, cards and accordion are good in some cases too

For Settings tabs are suites IMHO

ns8482e commented 2 years ago

UX varies by needs - your needs may differ than others, Converting to card/columns are fine when you have fewer fields in fewer tabs - but bad idea when you have lot of fields

I believe @deanmarcussen idea on organizing fields on tabs is better to utilize more real estate without vertical scroll.

OC’s flexible - you can use placement to achieve your UX needs.

sebastienros commented 2 years ago

Then try to update the user by leaving both required fields empty (from both tabs) and click update.

There should be another issue/PR about making this experience nicer by showing on which tabs the validation errors are, independently of whether cards/tabs is the default.