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

Add top level Multi Tenancy menu #12671

Merged
merged 5 commits into from
May 20, 2023
Merged

Add top level Multi Tenancy menu #12671

merged 5 commits into from
May 20, 2023

Conversation

ns8482e
Copy link
Contributor

@ns8482e ns8482e commented Oct 19, 2022

Add top level Multi Tenancy menu and Move tenants and Tenant feature profile under Multi Tenancy menu

Fixes #12670

image

@MikeAlhayek
Copy link
Member

@ns8482e
If you want to name it "Multi Tenancy", maybe it should be written "Multi-Tenancy". By what if you change the main menu to "Tenants" and then replace current "Tenants" with "Management" or "Manage Tenants"? So it would look like this

builder
    .Add(S["Tenants"], "after", tenants => tenants 
        .AddClass("tenants").Id("tenants")
        .Add(S["Manage"], S["Manage"].PrefixPosition(), manage => manage 
            .Action("Index", "Admin", new { area = "OrchardCore.Tenants" })
            .Permission(Permissions.ManageTenants)
            .LocalNav()
        )
    );

@Skrypt
Copy link
Contributor

Skrypt commented Oct 20, 2022

Moving the menu items makes sense to me. That's a top-level feature.

@ns8482e
Copy link
Contributor Author

ns8482e commented Oct 20, 2022

@MikeAlhayek I feel "Multi Tenancy" as top level menu as it represents a grouping of features related to multi-tenant, where as "Tenants" / "Tenant Features Profile" individual features, menu name kept it as is as we are used to with those menu names

@MikeAlhayek
Copy link
Member

I understand. IMO, "Tenants" delivers the same meaning in a shorter manner. But either way, its up to you

@ns8482e
Copy link
Contributor Author

ns8482e commented Oct 20, 2022

Some other CMS refer that top level menu as "Sites"

@ns8482e
Copy link
Contributor Author

ns8482e commented Oct 20, 2022

I am fine to use any top level menu name all agrees to

@MikeAlhayek
Copy link
Member

@ns8482e
Personally I think "Tenants" is better than "Sites" or "Multi-Tenancy". I'll leave to you and others.

@hishamco
Copy link
Member

I also see the term Sites in some CMSs, it's more clear for non technical users

@sebastienros sebastienros added this to the 1.x milestone May 2, 2023
@sebastienros
Copy link
Member

My suggestion

  • Multi-Tenancy
    • Feature Profiles
    • Tenants (because we already call it this way everywhere)

@MikeAlhayek
Copy link
Member

MikeAlhayek commented May 2, 2023

@ns8482e I make the changes @sebastienros mentioned. Feel free to merge this PR.

It may be a good idea to document the change in 1.7.0 release notes file.

@ns8482e ns8482e merged commit 21db149 into main May 20, 2023
@ns8482e ns8482e deleted the ns/#12670 branch May 20, 2023 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Tenants menu under Multi Tenancy menu
5 participants