FIX: consistent sidebar section external links (#22343)
Before this change, links which required full reload because they are not in ember routes like `/my/preferences` or links to docs like `/pub/*` were treated as real external links. Therefore, they were opening in self window or new tab based on user `external_links_in_new_tab` setting. To be consistent with behavior when full reload links are in the post, they are treated as internal and always open in the same window.
This commit is contained in:
parent
898e571a91
commit
3c019b1c0f
|
@ -8,13 +8,14 @@
|
||||||
@class={{this.section.dragCss}}
|
@class={{this.section.dragCss}}
|
||||||
>
|
>
|
||||||
{{#each this.section.links as |link|}}
|
{{#each this.section.links as |link|}}
|
||||||
{{#if link.external}}
|
{{#if link.externalOrFullReload}}
|
||||||
<Sidebar::SectionLink
|
<Sidebar::SectionLink
|
||||||
@shouldDisplay={{link.shouldDisplay}}
|
@shouldDisplay={{link.shouldDisplay}}
|
||||||
@linkName={{link.name}}
|
@linkName={{link.name}}
|
||||||
@content={{replace-emoji link.text}}
|
@content={{replace-emoji link.text}}
|
||||||
@prefixType="icon"
|
@prefixType="icon"
|
||||||
@prefixValue={{link.prefixValue}}
|
@prefixValue={{link.prefixValue}}
|
||||||
|
@fullReload={{link.fullReload}}
|
||||||
@href={{link.value}}
|
@href={{link.value}}
|
||||||
@class={{link.linkDragCss}}
|
@class={{link.linkDragCss}}
|
||||||
{{(if
|
{{(if
|
||||||
|
|
|
@ -1,10 +1,11 @@
|
||||||
{{#if @sectionLink.external}}
|
{{#if @sectionLink.externalOrFullReload}}
|
||||||
<Sidebar::SectionLink
|
<Sidebar::SectionLink
|
||||||
@shouldDisplay={{@sectionLink.shouldDisplay}}
|
@shouldDisplay={{@sectionLink.shouldDisplay}}
|
||||||
@linkName={{@sectionLink.name}}
|
@linkName={{@sectionLink.name}}
|
||||||
@content={{replace-emoji @sectionLink.text}}
|
@content={{replace-emoji @sectionLink.text}}
|
||||||
@prefixType="icon"
|
@prefixType="icon"
|
||||||
@prefixValue={{@sectionLink.prefixValue}}
|
@prefixValue={{@sectionLink.prefixValue}}
|
||||||
|
@fullReload={{@sectionLink.fullReload}}
|
||||||
@href={{@sectionLink.value}}
|
@href={{@sectionLink.value}}
|
||||||
/>
|
/>
|
||||||
{{else}}
|
{{else}}
|
||||||
|
|
|
@ -50,6 +50,9 @@ export default class SectionLink extends Component {
|
||||||
}
|
}
|
||||||
|
|
||||||
get target() {
|
get target() {
|
||||||
|
if (this.args.fullReload) {
|
||||||
|
return "_self";
|
||||||
|
}
|
||||||
return this.currentUser?.user_option?.external_links_in_new_tab
|
return this.currentUser?.user_option?.external_links_in_new_tab
|
||||||
? "_blank"
|
? "_blank"
|
||||||
: "_self";
|
: "_self";
|
||||||
|
|
|
@ -9,8 +9,13 @@ const MOUSE_DELAY = 250;
|
||||||
export default class SectionLink {
|
export default class SectionLink {
|
||||||
@tracked linkDragCss;
|
@tracked linkDragCss;
|
||||||
|
|
||||||
constructor({ external, icon, id, name, value }, section, router) {
|
constructor(
|
||||||
|
{ external, full_reload, icon, id, name, value },
|
||||||
|
section,
|
||||||
|
router
|
||||||
|
) {
|
||||||
this.external = external;
|
this.external = external;
|
||||||
|
this.fullReload = full_reload;
|
||||||
this.prefixValue = icon;
|
this.prefixValue = icon;
|
||||||
this.id = id;
|
this.id = id;
|
||||||
this.name = name;
|
this.name = name;
|
||||||
|
@ -18,7 +23,7 @@ export default class SectionLink {
|
||||||
this.value = value;
|
this.value = value;
|
||||||
this.section = section;
|
this.section = section;
|
||||||
|
|
||||||
if (!this.external) {
|
if (!this.externalOrFullReload) {
|
||||||
const routeInfoHelper = new RouteInfoHelper(router, value);
|
const routeInfoHelper = new RouteInfoHelper(router, value);
|
||||||
this.route = routeInfoHelper.route;
|
this.route = routeInfoHelper.route;
|
||||||
this.models = routeInfoHelper.models;
|
this.models = routeInfoHelper.models;
|
||||||
|
@ -30,6 +35,10 @@ export default class SectionLink {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
get externalOrFullReload() {
|
||||||
|
return this.external || this.fullReload;
|
||||||
|
}
|
||||||
|
|
||||||
@bind
|
@bind
|
||||||
didStartDrag(event) {
|
didStartDrag(event) {
|
||||||
// 0 represents left button of the mouse
|
// 0 represents left button of the mouse
|
||||||
|
|
|
@ -1,9 +1,13 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class SidebarUrlSerializer < ApplicationSerializer
|
class SidebarUrlSerializer < ApplicationSerializer
|
||||||
attributes :id, :name, :value, :icon, :external, :segment
|
attributes :id, :name, :value, :icon, :external, :full_reload, :segment
|
||||||
|
|
||||||
def external
|
def external
|
||||||
object.external? || object.full_reload?
|
object.external?
|
||||||
|
end
|
||||||
|
|
||||||
|
def full_reload
|
||||||
|
object.full_reload?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -6,6 +6,8 @@ describe "Custom sidebar sections", type: :system do
|
||||||
let(:section_modal) { PageObjects::Modals::SidebarSectionForm.new }
|
let(:section_modal) { PageObjects::Modals::SidebarSectionForm.new }
|
||||||
let(:sidebar) { PageObjects::Components::Sidebar.new }
|
let(:sidebar) { PageObjects::Components::Sidebar.new }
|
||||||
|
|
||||||
|
before { user.user_option.update!(external_links_in_new_tab: true) }
|
||||||
|
|
||||||
it "allows the user to create custom section" do
|
it "allows the user to create custom section" do
|
||||||
visit("/latest")
|
visit("/latest")
|
||||||
|
|
||||||
|
@ -40,7 +42,7 @@ describe "Custom sidebar sections", type: :system do
|
||||||
section_modal.save
|
section_modal.save
|
||||||
|
|
||||||
expect(sidebar).to have_section("My section")
|
expect(sidebar).to have_section("My section")
|
||||||
expect(sidebar).to have_section_link("My preferences")
|
expect(sidebar).to have_section_link("My preferences", target: "_self")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "allows the user to create custom section with /pub link" do
|
it "allows the user to create custom section with /pub link" do
|
||||||
|
@ -53,7 +55,7 @@ describe "Custom sidebar sections", type: :system do
|
||||||
section_modal.save
|
section_modal.save
|
||||||
|
|
||||||
expect(sidebar).to have_section("My section")
|
expect(sidebar).to have_section("My section")
|
||||||
expect(sidebar).to have_section_link("Published Page")
|
expect(sidebar).to have_section_link("Published Page", target: "_self")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "allows the user to create custom section with external link" do
|
it "allows the user to create custom section with external link" do
|
||||||
|
@ -76,7 +78,11 @@ describe "Custom sidebar sections", type: :system do
|
||||||
section_modal.save
|
section_modal.save
|
||||||
|
|
||||||
expect(sidebar).to have_section("My section")
|
expect(sidebar).to have_section("My section")
|
||||||
expect(sidebar).to have_section_link("Discourse Homepage", href: "https://discourse.org")
|
expect(sidebar).to have_section_link(
|
||||||
|
"Discourse Homepage",
|
||||||
|
href: "https://discourse.org",
|
||||||
|
target: "_blank",
|
||||||
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
it "allows the user to edit custom section" do
|
it "allows the user to edit custom section" do
|
||||||
|
|
|
@ -57,8 +57,8 @@ module PageObjects
|
||||||
has_css?(".#{SIDEBAR_SECTION_LINK_SELECTOR}--active", count: 1)
|
has_css?(".#{SIDEBAR_SECTION_LINK_SELECTOR}--active", count: 1)
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_section_link?(name, href: nil, active: false)
|
def has_section_link?(name, href: nil, active: false, target: nil)
|
||||||
section_link_present?(name, href: href, active: active, present: true)
|
section_link_present?(name, href: href, active: active, target: target, present: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_no_section_link?(name, href: nil, active: false)
|
def has_no_section_link?(name, href: nil, active: false)
|
||||||
|
@ -119,11 +119,12 @@ module PageObjects
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def section_link_present?(name, href: nil, active: false, present:)
|
def section_link_present?(name, href: nil, active: false, target: nil, present:)
|
||||||
attributes = { exact_text: name }
|
attributes = { exact_text: name }
|
||||||
attributes[:href] = href if href
|
attributes[:href] = href if href
|
||||||
attributes[:class] = SIDEBAR_SECTION_LINK_SELECTOR
|
attributes[:class] = SIDEBAR_SECTION_LINK_SELECTOR
|
||||||
attributes[:class] += "--active" if active
|
attributes[:class] += "--active" if active
|
||||||
|
attributes[:target] = target if target
|
||||||
page.public_send(present ? :has_link? : :has_no_link?, **attributes)
|
page.public_send(present ? :has_link? : :has_no_link?, **attributes)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue