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 target attribute to Menu #15636

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -33,6 +33,7 @@ public override IDisplayResult Edit(LinkAdminNode treeNode)
model.LinkText = treeNode.LinkText;
model.LinkUrl = treeNode.LinkUrl;
model.IconClass = treeNode.IconClass;
model.Target = treeNode.Target;

var permissions = await _adminMenuPermissionService.GetPermissionsAsync();

Expand All @@ -56,10 +57,11 @@ public override IDisplayResult Edit(LinkAdminNode treeNode)
public override async Task<IDisplayResult> UpdateAsync(LinkAdminNode treeNode, IUpdateModel updater)
{
var model = new LinkAdminNodeViewModel();
if (await updater.TryUpdateModelAsync(model, Prefix, x => x.LinkUrl, x => x.LinkText, x => x.IconClass, x => x.SelectedPermissionNames))
if (await updater.TryUpdateModelAsync(model, Prefix, x => x.LinkUrl, x => x.Target, x => x.LinkText, x => x.IconClass, x => x.SelectedPermissionNames))
{
treeNode.LinkText = model.LinkText;
treeNode.LinkUrl = model.LinkUrl;
treeNode.Target = model.Target;
treeNode.IconClass = model.IconClass;

var selectedPermissions = (model.SelectedPermissionNames == null ? [] : model.SelectedPermissionNames.Split(',', StringSplitOptions.RemoveEmptyEntries));
Expand Down
Expand Up @@ -59,6 +59,7 @@ public Task BuildNavigationAsync(MenuItem menuItem, NavigationBuilder builder, I

// Add the actual link.
itemBuilder.Url(nodeLinkUrl);
itemBuilder.Target(node.Target);
itemBuilder.Priority(node.Priority);
itemBuilder.Position(node.Position);

Expand Down
Expand Up @@ -12,6 +12,8 @@ public class LinkAdminNodeViewModel
[Required]
public string LinkUrl { get; set; }

public string Target { get; set; }

public string IconClass { get; set; }

public string SelectedPermissionNames { get; set; }
Expand Down
Expand Up @@ -24,6 +24,12 @@
<span class="hint">@T["The url of the link. A link will be shown only if it or one of their children have a url. The url will be relative to the root of the admin site"]</span>
</div>

<div class="mb-3">
<label asp-for="Target" class="form-label">@T["Target"]</label>
<input asp-for="Target" class="form-control" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Select menu instead of of a text box. The options can be

Value: Text
_blank: Opens the linked document in a new window or tab
_self Opens: the linked document in the same frame as it was clicked (this is default)
_parent: Opens the linked document in the parent frame
_top: Opens the linked document in the full body of the window

Copy link
Contributor Author

@hyzx86 hyzx86 Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This restricts users from using named window, such as those with a target attribute set to MyWorkflow, so clicking on any link if whose target is MyWorkflow will always use the same window

Not sure if OC has a text box similar to autocomplete, we can set the above options you mentioned as autocomplete options, while allowing the user to enter any custom content

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to create UI where anyone can use not just developers. Drop down menu will provide better user experience for the common use. What if we do a menu with "Other" options. When other is selected, we can show an input to provider the custom value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<span class="hint">@T["The target attribute of the A tag, see more:"] <a class="seedoc" target="_blank" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target">target</a></span>
</div>

<div class="mb-3">
<label asp-for="IconClass" class="form-label">@T["Icon"]</label>

Expand Down
Expand Up @@ -67,6 +67,7 @@ public override IDisplayResult Edit(HtmlMenuItemPart part)
{
model.Name = part.ContentItem.DisplayText;
model.Url = part.Url;
model.Target = part.Target;
model.Html = part.Html;
model.MenuItemPart = part;
});
Expand All @@ -81,6 +82,7 @@ public override async Task<IDisplayResult> UpdateAsync(HtmlMenuItemPart part, IU
part.ContentItem.DisplayText = model.Name;
part.Html = settings.SanitizeHtml ? _htmlSanitizerService.Sanitize(model.Html) : model.Html;
part.Url = model.Url;
part.Target = model.Target;

var urlToValidate = part.Url;

Expand Down
Expand Up @@ -59,6 +59,7 @@ public override IDisplayResult Edit(LinkMenuItemPart part)
{
model.Name = part.ContentItem.DisplayText;
model.Url = part.Url;
model.Target = part.Target;
model.MenuItemPart = part;
});
}
Expand All @@ -70,6 +71,7 @@ public override async Task<IDisplayResult> UpdateAsync(LinkMenuItemPart part, IU
if (await updater.TryUpdateModelAsync(model, Prefix))
{
part.Url = model.Url;
part.Target = model.Target;
part.ContentItem.DisplayText = model.Name;

// This code can be removed in a later release.
Expand Down
Expand Up @@ -10,6 +10,7 @@ public HtmlMenuItemQueryObjectType()
Name = "HtmlMenuItemPart";

Field(x => x.Url, nullable: true);
Field(x => x.Target, nullable: true);
Field(x => x.Html, nullable: true);
}
}
Expand Down
Expand Up @@ -9,6 +9,11 @@ public class HtmlMenuItemPart : ContentPart
/// </summary>
public string Url { get; set; }

/// <summary>
/// The target of the link to create.
/// </summary>
public string Target { get; set; }

/// <summary>
/// The raw html to display for this link.
/// </summary>
Expand Down
Expand Up @@ -12,5 +12,10 @@ public class LinkMenuItemPart : ContentPart
/// The url of the link to create.
/// </summary>
public string Url { get; set; }

/// <summary>
/// The target of the link to create.
/// </summary>
public string Target { get; set; }
}
}
Expand Up @@ -9,6 +9,8 @@ public class HtmlMenuItemPartEditViewModel

public string Url { get; set; }

public string Target { get; set; }

public string Html { get; set; }

[BindNever]
Expand Down
Expand Up @@ -9,6 +9,8 @@ public class LinkMenuItemPartEditViewModel

public string Url { get; set; }

public string Target { get; set; }

[BindNever]
public LinkMenuItemPart MenuItemPart { get; set; }
}
Expand Down
Expand Up @@ -15,6 +15,14 @@
</div>
</div>

<div class="@Orchard.GetWrapperClasses()">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Select

<label asp-for="Target" class="@Orchard.GetLabelClasses()">@T["Target"]</label>
<div class="@Orchard.GetEndClasses()">
<input asp-for="Target" class="form-control" />
<span class="hint">@T["The target attribute of the A tag, see more:"] <a class="seedoc" target="_blank" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target">target</a></span>
</div>
</div>

<div class="@Orchard.GetWrapperClasses()">
<label asp-for="Html" class="@Orchard.GetLabelClasses()">@T["Html"]</label>
<div class="@Orchard.GetEndClasses()">
Expand Down
Expand Up @@ -14,3 +14,11 @@
<input asp-for="Url" class="form-control" />
</div>
</div>

<div class="@Orchard.GetWrapperClasses()">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Select

<label asp-for="Target" class="@Orchard.GetLabelClasses()">@T["Target"]</label>
<div class="@Orchard.GetEndClasses()">
<input asp-for="Target" class="form-control" />
<span class="hint">@T["The target attribute of the A tag, see more:"] <a class="seedoc" target="_blank" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target">target</a></span>
</div>
</div>
Expand Up @@ -17,6 +17,12 @@
}

tag.Attributes["href"] = url;

if (!string.IsNullOrEmpty(htmlMenuItemPart.Target))
{
tag.Attributes["target"] = htmlMenuItemPart.Target;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate the value to make sure only valid values can be provided

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really not necessary, since the target attribute is freely definable, e.g. target="order_details" , and when the user clicks on a link it will always refresh the existing window as much as possible, instead of always opening a new one.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target

}

tag.InnerHtml.AppendHtml(Html.Raw(htmlMenuItemPart.Html));
}
@tag
Expand Up @@ -16,6 +16,11 @@
url = Url.Content(linkMenuItemPart.Url);
}

if (!string.IsNullOrEmpty(linkMenuItemPart.Target))
{
tag.Attributes["target"] = linkMenuItemPart.Target;
}

tag.Attributes["href"] = url;
tag.InnerHtml.Append(contentItem.DisplayText);
}
Expand Down
@@ -1 +1 @@
<a href="@(Model.Href ?? "#")">@Model.Text</a>
<a href="@(Model.Href ?? "#")" @(!string.IsNullOrEmpty(Model.Target) ? $"target=\"" + Model.Target + "\"" : string.Empty)>@Model.Text</a>
Expand Up @@ -5,4 +5,4 @@
Model.Metadata.Alternates.Add("NavigationItemText_Id__" + Model.Id);
}

<a href="@(Model.Href ?? "#")">@await DisplayAsync(Model)</a>
<a href="@(Model.Href ?? "#")" @(!string.IsNullOrEmpty(Model.Target) ? $"target=\"" + Model.Target + "\"" : string.Empty)>@await DisplayAsync(Model)</a>
Expand Up @@ -14,6 +14,11 @@
tag.Attributes["href"] = "#m" + Model.GetHashCode().ToString();
}

if (!string.IsNullOrEmpty(Model.Target))
{
tag.Attributes["target"] = Model.Target;
}

var prefix = "icon-class-";

// Extract classes that are not icons from Model.Classes
Expand Down
@@ -1,2 +1,2 @@
{% assign link = Model.ContentItem.Content.HtmlMenuItemPart %}
<a class="nav-link" href="{{ link.Url | href }}">{{ link.Html | raw }}</a>
<a class="nav-link" href="{{ link.Url | href }}"{% if link.Target %} target="{{link.Target}}" {% endif %}>{{ link.Html | raw }}</a>
@@ -1,5 +1,5 @@
{% assign link = Model.ContentItem.Content.LinkMenuItemPart %}
<a class="nav-link" href="{{ link.Url | href }}"
<a class="nav-link" href="{{ link.Url | href }}"{% if link.Target %} target="{{link.Target}}" {% endif %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

{% if link.Url contains "#" %}
data-bs-target="{{ link.Url | split: '/' | last }}"
{% endif %}>
Expand Down
Expand Up @@ -3,5 +3,5 @@
{% if Model.HasItems %}
<a href="{{ link.Url | href }}" class="nav-link px-lg-3 py-3 py-lg-4 dropdown-toggle" data-bs-toggle="dropdown">{{ link.Html | raw }}<b class="caret"></b></a>
{% else %}
<a class="nav-link px-lg-3 py-3 py-lg-4" href="{{ link.Url | href }}">{{ link.Html | raw }}</a>
<a class="nav-link px-lg-3 py-3 py-lg-4" href="{{ link.Url | href }}"{% if link.Target %} target="{{link.Target}}" {% endif %}>{{ link.Html | raw }}</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. With line at the end of the file

{% endif %}
Expand Up @@ -3,5 +3,5 @@
{% if Model.HasItems %}
<a href="{{ link.Url | href }}" class="nav-link px-lg-3 py-3 py-lg-4 dropdown-toggle" data-bs-toggle="dropdown">{{ Model.ContentItem.DisplayText }}<b class="caret"></b></a>
{% else %}
<a class="nav-link px-lg-3 py-3 py-lg-4" href="{{ link.Url | href }}">{{ Model.ContentItem.DisplayText }}</a>
<a class="nav-link px-lg-3 py-3 py-lg-4" href="{{ link.Url | href }}"{% if link.Target %} target="{{link.Target}}" {% endif %}>{{ Model.ContentItem.DisplayText }}</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

{% endif %}
Expand Up @@ -18,6 +18,12 @@
}

tag.Attributes["href"] = url;

if (!string.IsNullOrEmpty(htmlMenuItemPart.Target))
{
tag.Attributes["target"] = htmlMenuItemPart.Target;
}

tag.InnerHtml.AppendHtml(Html.Raw(htmlMenuItemPart.Html));

if (Model.Level == 0 && Model.HasItems)
Expand Down
5 changes: 5 additions & 0 deletions src/OrchardCore/OrchardCore.Navigation.Core/MenuItem.cs
Expand Up @@ -36,6 +36,11 @@ public MenuItem()
/// </summary>
public string Href { get; set; }

/// <summary>
/// The html target of the menu item.
/// </summary>
public string Target { get; set; }

/// <summary>
/// The optional url the menu item should link to.
/// </summary>
Expand Down
Expand Up @@ -68,6 +68,7 @@ private static async Task<dynamic> BuildMenuItemShapeAsync(dynamic shapeFactory,
var menuItemShape = (await shapeFactory.NavigationItem())
.Text(menuItem.Text)
.Href(menuItem.Href)
.Target(menuItem.Target)
.Url(menuItem.Url)
.LinkToFirstChild(menuItem.LinkToFirstChild)
.RouteValues(menuItem.RouteValues)
Expand Down
Expand Up @@ -45,6 +45,11 @@ public NavigationItemBuilder Url(string url)
_item.Url = url;
return this;
}
public NavigationItemBuilder Target(string target)
{
_item.Target = target;
return this;
}

public NavigationItemBuilder Culture(string culture)
{
Expand Down
Expand Up @@ -110,6 +110,7 @@ private static void Merge(List<MenuItem> items)
source.RouteValues = cursor.RouteValues;
source.Text = cursor.Text;
source.Url = cursor.Url;
source.Target = cursor.Target;

source.Permissions.Clear();
source.Permissions.AddRange(cursor.Permissions);
Expand All @@ -133,6 +134,7 @@ private static void Merge(List<MenuItem> items)
source.RouteValues = cursor.RouteValues;
source.Text = cursor.Text;
source.Url = cursor.Url;
source.Target = cursor.Target;

source.Permissions.Clear();
source.Permissions.AddRange(cursor.Permissions);
Expand Down
4 changes: 4 additions & 0 deletions src/docs/releases/1.9.0.md
Expand Up @@ -123,6 +123,10 @@ The `OrchardCore.Email` module has undergone a refactoring process with no break
- If you were using the `OrchardCore_Email` configuration key to set up the SMTP provider for all tenants, please update the configuration key to `OrchardCore_Email_Smtp`. The `OrchardCore_Email` key continues to work but will be deprecated in a future release.
- A new email provider was added to allow you to send email using Azure Communication Services Email. Click [here](../reference/modules/Email.Azure/README.md) to read more about the ACS module.

### Menu

`Menus` and `AdminMenus` now support specifying the target property.

### Admin Menu

The admin menu has undergone performance enhancements, and new helpers have been added. When incorporating `INavigationProvider` in your project, you can now utilize `NavigationHelper.IsAdminMenu(name)` instead of the previous approach using `string.Equals(name, "admin", StringComparison.OrdinalIgnoreCase)`. Moreover, when passing route values to an action, it is advised to store them in a constant variable. An illustrative example is provided below.
Expand Down