FIX: improvements for user custom sections (#20190)

Improvements for this PR: https://github.com/discourse/discourse/pull/20057

What was fixed:
- [x] Use ember transitions instead of full reload
- [x] Link was inaccurately kept active
- [x] "+ save" renamed to just "save"
- [x] Render emojis in link name
- [x] UI to set icon
- [x] Delete link is trash icon instead of "x"
- [x] Add another link to on the left and rewording
- [x] Raname "link name" -> "name", "points to" ->  link
- [x] Add limits to fields
- [x] Move add section button to the bottom
This commit is contained in:
Krzysztof Kotlarek 2023-02-08 11:45:34 +11:00 committed by GitHub
parent 9a45b59fb5
commit 6e1f3e0023
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 177 additions and 127 deletions

View File

@ -3,6 +3,13 @@
<div class="sidebar-footer-actions">
<PluginOutlet @name="sidebar-footer-actions" />
<DButton
@icon="plus"
@action={{action this.addSection}}
@class="btn-flat add-section"
@title="sidebar.sections.custom.add"
/>
{{#if
(or
this.site.mobileView

View File

@ -1,6 +1,8 @@
import Component from "@glimmer/component";
import { getOwner } from "discourse-common/lib/get-owner";
import { inject as service } from "@ember/service";
import { action } from "@ember/object";
import showModal from "discourse/lib/show-modal";
export default class SidebarFooter extends Component {
@service site;
@ -9,4 +11,9 @@ export default class SidebarFooter extends Component {
get capabilities() {
return getOwner(this).lookup("capabilities:main");
}
@action
addSection() {
showModal("sidebar-section-form");
}
}

View File

@ -7,7 +7,7 @@
<a
href={{@href}}
rel="noopener noreferrer"
target={{or @target "_blank"}}
target="_blank"
class={{this.classNames}}
title={{@title}}
>

View File

@ -1,5 +1,5 @@
<div class="sidebar-custom-sections">
{{#each this.currentUser.sidebarSections as |section|}}
{{#each this.sections as |section|}}
<Sidebar::Section
@sectionName={{section.slug}}
@headerLinkText={{section.title}}
@ -15,19 +15,14 @@
{{#each section.links as |link|}}
<Sidebar::SectionLink
@linkName={{link.name}}
@href={{link.value}}
@target="_self"
@content={{link.name}}
@class={{link.class}}
@route={{link.route}}
@models={{link.models}}
@query={{link.query}}
@content={{replace-emoji link.name}}
@prefixType="icon"
@prefixValue={{link.icon}}
/>
{{/each}}
</Sidebar::Section>
{{/each}}
<DButton
@icon="plus"
@action={{action this.addSection}}
@class="btn-flat add-section"
@title="sidebar.sections.custom.add"
/>
</div>

View File

@ -2,30 +2,26 @@ import Component from "@glimmer/component";
import { action } from "@ember/object";
import showModal from "discourse/lib/show-modal";
import { inject as service } from "@ember/service";
import RouteInfoHelper from "discourse/lib/sidebar/route-info-helper";
export default class SidebarUserCustomSections extends Component {
@service currentUser;
constructor() {
super(...arguments);
this.sections.forEach((section) => {
section.links.forEach((link) => {
link.class = window.location.pathname === link.value ? "active" : "";
});
});
}
@service router;
get sections() {
return this.currentUser.sidebar_sections || [];
this.currentUser.sidebarSections.forEach((section) => {
section.links.forEach((link) => {
const routeInfoHelper = new RouteInfoHelper(this.router, link.value);
link.route = routeInfoHelper.route;
link.models = routeInfoHelper.models;
link.query = routeInfoHelper.query;
});
});
return this.currentUser.sidebarSections;
}
@action
editSection(section) {
showModal("sidebar-section-form", { model: section });
}
addSection() {
showModal("sidebar-section-form");
}
}

View File

@ -26,7 +26,7 @@ class Section {
}
get validTitle() {
return !isEmpty(this.title);
return !isEmpty(this.title) && this.title.length <= 30;
}
get titleCssClass() {
@ -35,12 +35,14 @@ class Section {
}
class SectionLink {
@tracked icon;
@tracked name;
@tracked value;
@tracked _destroy;
constructor({ router, name, value, id }) {
constructor({ router, icon, name, value, id }) {
this.router = router;
this.icon = icon || "link";
this.name = name;
this.value = value ? `${this.protocolAndHost}${value}` : value;
this.id = id;
@ -55,11 +57,19 @@ class SectionLink {
}
get valid() {
return this.validName && this.validValue;
return this.validIcon && this.validName && this.validValue;
}
get validIcon() {
return !isEmpty(this.icon) && this.icon.length <= 40;
}
get iconCssClass() {
return this.icon === undefined || this.validIcon ? "" : "warning";
}
get validName() {
return !isEmpty(this.name);
return !isEmpty(this.name) && this.name.length <= 80;
}
get nameCssClass() {
@ -71,6 +81,7 @@ class SectionLink {
!isEmpty(this.value) &&
(this.value.startsWith(this.protocolAndHost) ||
this.value.startsWith("/")) &&
this.value.length <= 200 &&
this.path &&
this.router.recognize(this.path).name !== "unknown"
);
@ -106,6 +117,7 @@ export default Controller.extend(ModalFunctionality, {
(link) =>
new SectionLink({
router: this.router,
icon: link.icon,
name: link.name,
value: link.value,
id: link.id,
@ -130,6 +142,7 @@ export default Controller.extend(ModalFunctionality, {
title: this.model.title,
links: this.model.links.map((link) => {
return {
icon: link.icon,
name: link.name,
value: link.path,
};
@ -158,6 +171,7 @@ export default Controller.extend(ModalFunctionality, {
links: this.model.links.map((link) => {
return {
id: link.id,
icon: link.icon,
name: link.name,
value: link.path,
_destroy: link._destroy,

View File

@ -1,45 +1,9 @@
import BaseCommunitySectionLink from "discourse/lib/sidebar/base-community-section-link";
import RouteInfoHelper from "discourse/lib/sidebar/route-info-helper";
export let customSectionLinks = [];
export let secondaryCustomSectionLinks = [];
class RouteInfoHelper {
constructor(router, url) {
this.routeInfo = router.recognize(url);
}
get route() {
return this.routeInfo.name;
}
get models() {
return this.#getParameters;
}
get query() {
return this.routeInfo.queryParams;
}
/**
* Extracted from https://github.com/emberjs/rfcs/issues/658
* Retrieves all parameters for a `RouteInfo` object and its parents in
* correct oder, so that you can pass them to e.g.
* `transitionTo(routeName, ...params)`.
*/
get #getParameters() {
let allParameters = [];
let current = this.routeInfo;
do {
const { params, paramNames } = current;
const currentParameters = paramNames.map((n) => params[n]);
allParameters = [...currentParameters, ...allParameters];
} while ((current = current.parent));
return allParameters;
}
}
/**
* Appends an additional section link to the Community section under the "More..." links drawer.
*

View File

@ -0,0 +1,36 @@
export default class RouteInfoHelper {
constructor(router, url) {
this.routeInfo = router.recognize(url);
}
get route() {
return this.routeInfo.name;
}
get models() {
return this.#getParameters;
}
get query() {
return this.routeInfo.queryParams;
}
/**
* Extracted from https://github.com/emberjs/rfcs/issues/658
* Retrieves all parameters for a `RouteInfo` object and its parents in
* correct oder, so that you can pass them to e.g.
* `transitionTo(routeName, ...params)`.
*/
get #getParameters() {
let allParameters = [];
let current = this.routeInfo;
do {
const { params, paramNames } = current;
const currentParameters = paramNames.map((n) => params[n]);
allParameters = [...currentParameters, ...allParameters];
} while ((current = current.parent));
return allParameters;
}
}

View File

@ -17,6 +17,18 @@
</div>
{{#each this.activeLinks as |link|}}
<div class="row-wrapper">
<div class="input-group">
<label for="link-name">{{i18n
"sidebar.sections.custom.links.icon"
}}</label>
<IconPicker
@name="icon"
@value={{link.icon}}
@options={{hash maximum=1}}
class={{link.iconCssClass}}
@onChange={{action (mut link.icon)}}
/>
</div>
<div class="input-group">
<label for="link-name">{{i18n
"sidebar.sections.custom.links.name"
@ -31,7 +43,7 @@
</div>
<div class="input-group">
<label for="link-url">{{i18n
"sidebar.sections.custom.links.points_to"
"sidebar.sections.custom.links.value"
}}</label>
<Input
name="link-url"
@ -42,7 +54,7 @@
/>
</div>
<DButton
@icon="times"
@icon="trash-alt"
@action={{action "deleteLink" link}}
@class="btn-flat delete-link"
@title="sidebar.sections.custom.links.delete"
@ -53,6 +65,7 @@
@action={{action "addLink"}}
@class="btn-flat btn-text add-link"
@title="sidebar.sections.custom.links.add"
@icon="plus"
@label="sidebar.sections.custom.links.add"
/>
</form>
@ -63,7 +76,6 @@
@id="save-section"
@action={{action "save"}}
@class="btn-primary"
@icon="plus"
@label="sidebar.sections.custom.save"
@disabled={{not this.model.valid}}
/>

View File

@ -86,6 +86,17 @@
transition-delay: 0s;
}
}
.sidebar-footer-wrapper {
.btn-flat.add-section {
padding: 0.25em 0.4em;
&:hover {
background: var(--d-sidebar-highlight-color);
svg {
color: var(--primary-medium);
}
}
}
}
}
.sidebar-hamburger-dropdown {
@ -112,44 +123,17 @@
}
.sidebar-custom-sections {
.btn-flat.add-section {
margin-left: calc(var(--d-sidebar-section-link-prefix-width) / 2);
margin-right: calc(var(--d-sidebar-section-link-prefix-width) / 2);
width: calc(100% - var(--d-sidebar-section-link-prefix-width));
svg {
height: 0.75em;
width: 0.75em;
padding-left: 0.5em;
padding-right: 0.5em;
}
&:before,
&:after {
content: "";
flex: 1 1;
border-bottom: 1px solid var(--primary-low-mid);
margin: auto;
}
&:hover {
background: var(--d-sidebar-highlight-color);
border-radius: 5px;
&:before,
&:after {
border-bottom: 1px solid var(--primary-high);
}
}
}
a.sidebar-section-link {
padding-left: calc(
var(--d-sidebar-section-link-prefix-width) +
var(--d-sidebar-section-link-prefix-margin-right) +
var(--d-sidebar-row-horizontal-padding)
);
.sidebar-section-wrapper {
padding-bottom: 0;
}
}
.sidebar-section-form-modal {
.modal-inner-container {
width: var(--modal-max-width);
}
form {
margin-bottom: 0;
}
input {
width: 100%;
}
@ -158,7 +142,7 @@
}
.row-wrapper {
display: grid;
grid-template-columns: auto auto 2em;
grid-template-columns: auto auto auto 2em;
gap: 1em;
margin-top: 1em;
}
@ -169,13 +153,20 @@
margin-right: 1em;
}
.btn-flat.add-link {
float: right;
margin-top: 1em;
margin-right: -0.5em;
margin-left: -0.65em;
&:active,
&:focus {
background: none;
}
svg {
color: var(--tertiary);
width: 0.75em;
height: 0.75em;
}
&:hover svg {
color: var(--tertiary-hover);
}
}
.modal-footer {
display: flex;

View File

@ -42,7 +42,7 @@ class SidebarSectionsController < ApplicationController
end
def links_params
params.permit(links: %i[name value id _destroy])["links"]
params.permit(links: %i[icon name value id _destroy])["links"]
end
def check_if_member_of_group

View File

@ -10,7 +10,7 @@ class SidebarSection < ActiveRecord::Base
accepts_nested_attributes_for :sidebar_urls, allow_destroy: true
validates :title, presence: true, uniqueness: { scope: %i[user_id] }
validates :title, presence: true, uniqueness: { scope: %i[user_id] }, length: { maximum: 30 }
end
# == Schema Information
@ -19,7 +19,7 @@ end
#
# id :bigint not null, primary key
# user_id :integer not null
# title :string not null
# title :string(30) not null
# created_at :datetime not null
# updated_at :datetime not null
#

View File

@ -1,8 +1,10 @@
# frozen_string_literal: true
class SidebarUrl < ActiveRecord::Base
validates :name, presence: true
validates :value, presence: true
validates :icon, presence: true, length: { maximum: 40 }
validates :name, presence: true, length: { maximum: 80 }
validates :value, presence: true, length: { maximum: 200 }
validate :path_validator
def path_validator
@ -20,8 +22,9 @@ end
# Table name: sidebar_urls
#
# id :bigint not null, primary key
# name :string not null
# value :string not null
# name :string(80) not null
# value :string(200) not null
# created_at :datetime not null
# updated_at :datetime not null
# icon :string(40) not null
#

View File

@ -4368,9 +4368,10 @@ en:
delete: "Delete"
delete_confirm: "Are you sure you want to delete this section?"
links:
name: "Link name"
points_to: "Points to"
add: "Add link"
icon: "Icon"
name: "Name"
value: "Link"
add: "Add another link"
delete: "Delete link"
about:
header_link_text: "About"

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddIconToSidebarUrls < ActiveRecord::Migration[7.0]
def change
add_column :sidebar_urls, :icon, :string
end
end

View File

@ -0,0 +1,10 @@
# frozen_string_literal: true
class AddLimitsToSidebarSectionsAndSidebarUrls < ActiveRecord::Migration[7.0]
def change
change_column :sidebar_sections, :title, :string, limit: 30, null: false
change_column :sidebar_urls, :icon, :string, limit: 40, null: false
change_column :sidebar_urls, :name, :string, limit: 80, null: false
change_column :sidebar_urls, :value, :string, limit: 200, null: false
end
end

View File

@ -1,6 +1,7 @@
# frozen_string_literal: true
Fabricator(:sidebar_url) do
icon "link"
name "tags"
value "/tags"
end

View File

@ -2,7 +2,11 @@
RSpec.describe SidebarUrl do
it "validates path" do
expect(SidebarUrl.new(name: "categories", value: "/categories").valid?).to eq(true)
expect(SidebarUrl.new(name: "categories", value: "/invalid_path").valid?).to eq(false)
expect(SidebarUrl.new(icon: "link", name: "categories", value: "/categories").valid?).to eq(
true,
)
expect(SidebarUrl.new(icon: "link", name: "categories", value: "/invalid_path").valid?).to eq(
false,
)
end
end

View File

@ -16,8 +16,8 @@ RSpec.describe SidebarSectionsController do
params: {
title: "custom section",
links: [
{ name: "categories", value: "/categories" },
{ name: "tags", value: "/tags" },
{ icon: "link", name: "categories", value: "/categories" },
{ icon: "link", name: "tags", value: "/tags" },
],
}
@ -30,8 +30,8 @@ RSpec.describe SidebarSectionsController do
params: {
title: "custom section",
links: [
{ name: "categories", value: "/categories" },
{ name: "tags", value: "/tags" },
{ icon: "link", name: "categories", value: "/categories" },
{ icon: "address-book", name: "tags", value: "/tags" },
],
}
@ -43,8 +43,10 @@ RSpec.describe SidebarSectionsController do
expect(sidebar_section.title).to eq("custom section")
expect(sidebar_section.user).to eq(user)
expect(sidebar_section.sidebar_urls.count).to eq(2)
expect(sidebar_section.sidebar_urls.first.icon).to eq("link")
expect(sidebar_section.sidebar_urls.first.name).to eq("categories")
expect(sidebar_section.sidebar_urls.first.value).to eq("/categories")
expect(sidebar_section.sidebar_urls.second.icon).to eq("address-book")
expect(sidebar_section.sidebar_urls.second.name).to eq("tags")
expect(sidebar_section.sidebar_urls.second.value).to eq("/tags")
end
@ -67,8 +69,8 @@ RSpec.describe SidebarSectionsController do
params: {
title: "custom section edited",
links: [
{ id: sidebar_url_1.id, name: "latest", value: "/latest" },
{ id: sidebar_url_2.id, name: "tags", value: "/tags", _destroy: "1" },
{ icon: "link", id: sidebar_url_1.id, name: "latest", value: "/latest" },
{ icon: "link", id: sidebar_url_2.id, name: "tags", value: "/tags", _destroy: "1" },
],
}
@ -89,7 +91,7 @@ RSpec.describe SidebarSectionsController do
put "/sidebar_sections/#{sidebar_section_2.id}.json",
params: {
title: "custom section edited",
links: [{ id: sidebar_url_3.id, name: "takeover", value: "/categories" }],
links: [{ icon: "link", id: sidebar_url_3.id, name: "takeover", value: "/categories" }],
}
expect(response.status).to eq(403)
@ -106,7 +108,7 @@ RSpec.describe SidebarSectionsController do
put "/sidebar_sections/#{sidebar_section.id}.json",
params: {
title: "custom section edited",
links: [{ id: sidebar_url_3.id, name: "takeover", value: "/categories" }],
links: [{ icon: "link", id: sidebar_url_3.id, name: "takeover", value: "/categories" }],
}
expect(response.status).to eq(404)