OrchardCore icon indicating copy to clipboard operation
OrchardCore copied to clipboard

Move all module settings to Configuration > Settings

Open MikeAlhayek opened this issue 2 years ago • 6 comments

Fix #11942 Fix #11773 Closes #11972

Move all settings menu items into to one central place (i.e., Configuration > Settings)

Also, make the spacing/formatting consistent in all of the AdminManu classes for consistency and a bit more better readability.

Here is how the menu looks when all the feature the provide settings are enabled

image

MikeAlhayek avatar Jul 02 '22 01:07 MikeAlhayek

Please don't request the review from many unless there's a reason for it, regarding the PR I forgot to push my PR regarding the issue I created, but it's already on my local. For the new settings changes that you made let us see if it's fine to move them into configuration menu

Last thing please don't change anything else unrelated to PR, I appreciate the effort you did, bu it's good to focus on fixing the issue itself

Thanks

hishamco avatar Jul 02 '22 15:07 hishamco

@hishamco The request for review to multiple people was automatic as Github added them as "code owners".

Skrypt avatar Jul 02 '22 15:07 Skrypt

I see, but I think you can't remove them before push the PR

hishamco avatar Jul 02 '22 16:07 hishamco

@CrestApps I will push a PR for #11773 and merge it then, I will keep this open because what I saw there's a breaking changes in the translation, so please fix the conflict one I merge my PR

Thanks

hishamco avatar Jul 08 '22 12:07 hishamco

I'm on half of the reviewing the changes ;) just few notes, please remove unrelated changes to this PR. Fix formating and remove extra whitespaces

I am going to await the outcome of the outcome of the conversation in the issue #11942. I we'll make the necessary changes then. On a side note, some of the "unrelated" changes you suggested is just general clean up just to unify the code in all of the AdminMenu classes.

MikeAlhayek avatar Jul 08 '22 14:07 MikeAlhayek

image

Skrypt avatar Jul 21 '22 17:07 Skrypt

@sebastienros maybe this is something we should discuss on Tuesday to either close it or approve the change... or request a vote on.

MikeAlhayek avatar Jan 05 '23 00:01 MikeAlhayek

Please vote on the first issue with 👍 and 👎 to decide if we merge or close this PR.

sebastienros avatar Jan 10 '23 20:01 sebastienros

Current menus with settings: image image image image

agriffard avatar Jan 10 '23 20:01 agriffard

tbh, I expect search settings to be under "Search Module", Security Settings to be under Security, Localization Settings under Localization module etc... This way custom modules will "keep" their settings inside module area and would be easier for user to locate them What is proposed, I am afraid it will duplicate the modules structure under "configuration">"settings"

urbanit avatar Jan 10 '23 21:01 urbanit

One down vote, nobody else cares.

sebastienros avatar May 02 '23 19:05 sebastienros