UX: update `/new` toggle styles, class names (#23154)

* Minor style adjustments
* Removes "all" count because it's redundant to the count on New
* Updates generic class names with -- modifier to follow BEM and help avoid class name collisions
* Hides the toggle when bulk select is enabled (the UI ends up being too busy)
This commit is contained in:
Kris 2023-08-20 21:34:12 -04:00 committed by GitHub
parent b03c26ebf5
commit 2a49757f35
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 45 additions and 66 deletions

View File

@ -3,11 +3,11 @@ import Component from "@glimmer/component";
export default class NewListHeaderControlsWrapper extends Component {
click(e) {
const target = e.target;
if (target.closest("button.topics-replies-toggle.all")) {
if (target.closest("button.topics-replies-toggle.--all")) {
this.args.changeNewListSubset(null);
} else if (target.closest("button.topics-replies-toggle.topics")) {
} else if (target.closest("button.topics-replies-toggle.--topics")) {
this.args.changeNewListSubset("topics");
} else if (target.closest("button.topics-replies-toggle.replies")) {
} else if (target.closest("button.topics-replies-toggle.--replies")) {
this.args.changeNewListSubset("replies");
}
}

View File

@ -186,11 +186,11 @@ export default Component.extend(LoadMore, {
});
onClick("button.topics-replies-toggle", (element) => {
if (element.classList.contains("all")) {
if (element.classList.contains("--all")) {
this.changeNewListSubset(null);
} else if (element.classList.contains("topics")) {
} else if (element.classList.contains("--topics")) {
this.changeNewListSubset("topics");
} else if (element.classList.contains("replies")) {
} else if (element.classList.contains("--replies")) {
this.changeNewListSubset("replies");
}
this.rerender();

View File

@ -1,13 +1,13 @@
{{#if view.staticLabel}}
<span class="static-label">{{view.staticLabel}}</span>
{{else}}
<button class="topics-replies-toggle all{{#if view.allActive}} active{{/if}}">
{{view.allButtonLabel}}
<button class="topics-replies-toggle --all{{#if view.allActive}} active{{/if}}">
{{i18n "filters.new.all"}}
</button>
<button class="topics-replies-toggle topics{{#if view.topicsActive}} active{{/if}}">
<button class="topics-replies-toggle --topics{{#if view.topicsActive}} active{{/if}}">
{{view.topicsButtonLabel}}
</button>
<button class="topics-replies-toggle replies{{#if view.repliesActive}} active{{/if}}">
<button class="topics-replies-toggle --replies{{#if view.repliesActive}} active{{/if}}">
{{view.repliesButtonLabel}}
</button>
{{/if}}

View File

@ -13,11 +13,13 @@
</span>
{{/if ~}}
{{/if ~}}
{{~#if view.showTopicsAndRepliesToggle}}
{{raw "list/new-list-header-controls" current=newListSubset newRepliesCount=newRepliesCount newTopicsCount=newTopicsCount}}
{{else}}
<span>{{view.localizedName}}</span>
{{/if ~}}
{{~#unless bulkSelectEnabled}}
{{~#if view.showTopicsAndRepliesToggle}}
{{raw "list/new-list-header-controls" current=newListSubset newRepliesCount=newRepliesCount newTopicsCount=newTopicsCount}}
{{else}}
<span>{{view.localizedName}}</span>
{{/if ~}}
{{/unless ~}}
{{~#if view.isSorting}}
{{d-icon view.sortIcon}}
{{/if ~}}

View File

@ -18,16 +18,6 @@ export default EmberObject.extend({
return !this.topicsActive && !this.repliesActive;
},
@discourseComputed
allButtonLabel() {
const count = this.newRepliesCount + this.newTopicsCount;
if (count > 0) {
return I18n.t("filters.new.all_with_count", { count });
} else {
return I18n.t("filters.new.all");
}
},
@discourseComputed
repliesButtonLabel() {
if (this.newRepliesCount > 0) {

View File

@ -39,15 +39,16 @@
</a>
{{/if}}
{{/if}}
{{#if this.showTopicsAndRepliesToggle}}
<NewListHeaderControlsWrapper
@current={{this.model.params.subset}}
@newRepliesCount={{this.newRepliesCount}}
@newTopicsCount={{this.newTopicsCount}}
@changeNewListSubset={{route-action "changeNewListSubset"}}
/>
{{/if}}
{{#unless this.bulkSelectEnabled}}
{{#if this.showTopicsAndRepliesToggle}}
<NewListHeaderControlsWrapper
@current={{this.model.params.subset}}
@newRepliesCount={{this.newRepliesCount}}
@newTopicsCount={{this.newTopicsCount}}
@changeNewListSubset={{route-action "changeNewListSubset"}}
/>
{{/if}}
{{/unless}}
{{#if this.hasTopics}}
<TopicList
@ascending={{this.ascending}}

View File

@ -126,15 +126,18 @@
</div>
{{/if}}
{{/if}}
{{#if (and this.showTopicsAndRepliesToggle this.site.mobileView)}}
<NewListHeaderControlsWrapper
@current={{this.subset}}
@newRepliesCount={{this.newRepliesCount}}
@newTopicsCount={{this.newTopicsCount}}
@changeNewListSubset={{action "changeNewListSubset"}}
/>
{{/if}}
{{#unless this.bulkSelectEnabled}}
{{#if
(and this.showTopicsAndRepliesToggle this.site.mobileView)
}}
<NewListHeaderControlsWrapper
@current={{this.subset}}
@newRepliesCount={{this.newRepliesCount}}
@newTopicsCount={{this.newTopicsCount}}
@changeNewListSubset={{action "changeNewListSubset"}}
/>
{{/if}}
{{/unless}}
{{#if this.list.topics}}
<TopicList
@topics={{this.list.topics}}

View File

@ -39,6 +39,7 @@
button.bulk-select {
padding: 0;
margin-right: 0.5em;
line-height: var(--line-height-large);
}
@ -59,8 +60,8 @@
background: none;
border: none;
line-height: var(--line-height-large);
min-height: 30px;
padding-left: 0.5em;
padding-right: 0.5em;
&.active {
background: var(--quaternary);
color: var(--secondary);

View File

@ -88,7 +88,6 @@ describe "New topic list", type: :system do
new_topic_with_tag,
].each { |topic| expect(topic_list).to have_topic(topic) }
expect(tabs_toggle.all_tab).to have_count(6)
expect(tabs_toggle.replies_tab).to have_count(3)
expect(tabs_toggle.topics_tab).to have_count(3)
@ -124,7 +123,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_inactive
expect(tabs_toggle.topics_tab).to be_active
expect(tabs_toggle.all_tab).to have_count(6)
expect(tabs_toggle.replies_tab).to have_count(3)
expect(tabs_toggle.topics_tab).to have_count(3)
@ -144,7 +142,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_active
expect(tabs_toggle.topics_tab).to be_inactive
expect(tabs_toggle.all_tab).to have_count(6)
expect(tabs_toggle.replies_tab).to have_count(3)
expect(tabs_toggle.topics_tab).to have_count(3)
@ -170,19 +167,16 @@ describe "New topic list", type: :system do
it "live-updates the counts shown on the tabs" do
visit("/new")
expect(tabs_toggle.all_tab).to have_count(6)
expect(tabs_toggle.replies_tab).to have_count(3)
expect(tabs_toggle.topics_tab).to have_count(3)
TopicUser.update_last_read(user, new_reply_in_category.id, 2, 1, 1)
expect(tabs_toggle.all_tab).to have_count(5)
expect(tabs_toggle.replies_tab).to have_count(2)
expect(tabs_toggle.topics_tab).to have_count(3)
TopicUser.update_last_read(user, new_topic.id, 1, 1, 1)
expect(tabs_toggle.all_tab).to have_count(4)
expect(tabs_toggle.replies_tab).to have_count(2)
expect(tabs_toggle.topics_tab).to have_count(2)
end
@ -198,7 +192,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_inactive
expect(tabs_toggle.topics_tab).to be_inactive
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
end
@ -214,7 +207,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_inactive
expect(tabs_toggle.topics_tab).to be_active
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
@ -234,7 +226,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_active
expect(tabs_toggle.topics_tab).to be_inactive
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
@ -262,13 +253,11 @@ describe "New topic list", type: :system do
visit("/c/#{category.slug}/#{category.id}/l/new")
expect(tabs_toggle.all_tab).to have_count(3)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(2)
TopicUser.update_last_read(user, new_topic_in_category.id, 1, 1, 1)
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
end
@ -286,7 +275,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_inactive
expect(tabs_toggle.topics_tab).to be_inactive
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
end
@ -302,7 +290,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_inactive
expect(tabs_toggle.topics_tab).to be_active
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
@ -321,7 +308,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_active
expect(tabs_toggle.topics_tab).to be_inactive
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
@ -347,13 +333,11 @@ describe "New topic list", type: :system do
visit("/tag/#{tag.name}/l/new")
expect(tabs_toggle.all_tab).to have_count(3)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(2)
TopicUser.update_last_read(user, new_topic_with_tag.id, 1, 1, 1)
expect(tabs_toggle.all_tab).to have_count(2)
expect(tabs_toggle.replies_tab).to have_count(1)
expect(tabs_toggle.topics_tab).to have_count(1)
end
@ -389,7 +373,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_visible
expect(tabs_toggle.topics_tab).to be_visible
expect(tabs_toggle.all_tab).to have_count(3)
expect(tabs_toggle.replies_tab).to have_count(3)
expect(tabs_toggle.topics_tab).to have_count(0)
end
@ -411,7 +394,6 @@ describe "New topic list", type: :system do
expect(tabs_toggle.replies_tab).to be_visible
expect(tabs_toggle.topics_tab).to be_visible
expect(tabs_toggle.all_tab).to have_count(3)
expect(tabs_toggle.replies_tab).to have_count(0)
expect(tabs_toggle.topics_tab).to have_count(3)
end

View File

@ -5,9 +5,9 @@ module PageObjects
class NewTopicListToggle < PageObjects::Components::Base
COMMON_SELECTOR = ".topics-replies-toggle"
ALL_SELECTOR = "#{COMMON_SELECTOR}.all"
REPLIES_SELECTOR = "#{COMMON_SELECTOR}.replies"
TOPICS_SELECTOR = "#{COMMON_SELECTOR}.topics"
ALL_SELECTOR = "#{COMMON_SELECTOR}.--all"
REPLIES_SELECTOR = "#{COMMON_SELECTOR}.--replies"
TOPICS_SELECTOR = "#{COMMON_SELECTOR}.--topics"
def not_rendered?
has_no_css?(COMMON_SELECTOR)