diff --git a/app/assets/javascripts/discourse/app/components/bootstrap-mode-notice.hbs b/app/assets/javascripts/discourse/app/components/bootstrap-mode-notice.hbs index 6b239ea0144..f56b9a969c5 100644 --- a/app/assets/javascripts/discourse/app/components/bootstrap-mode-notice.hbs +++ b/app/assets/javascripts/discourse/app/components/bootstrap-mode-notice.hbs @@ -11,6 +11,7 @@ @priority={{900}} @titleText={{i18n "user_tips.admin_guide.title"}} @contentHtml={{this.userTipContent}} + @showSkipButton={{true}} /> {{/if}} diff --git a/app/assets/javascripts/discourse/app/components/header/user-dropdown/notifications.gjs b/app/assets/javascripts/discourse/app/components/header/user-dropdown/notifications.gjs index c7ecc939623..ed507361dad 100644 --- a/app/assets/javascripts/discourse/app/components/header/user-dropdown/notifications.gjs +++ b/app/assets/javascripts/discourse/app/components/header/user-dropdown/notifications.gjs @@ -54,6 +54,7 @@ export default class Notifications extends Component { @placement="bottom-end" @titleText={{i18n "user_tips.first_notification.title"}} @contentText={{i18n "user_tips.first_notification.content"}} + @showSkipButton={{true}} /> {{/if}} diff --git a/app/assets/javascripts/discourse/app/components/topic-timeline.hbs b/app/assets/javascripts/discourse/app/components/topic-timeline.hbs index 4670c1a10f0..9c5a6e61218 100644 --- a/app/assets/javascripts/discourse/app/components/topic-timeline.hbs +++ b/app/assets/javascripts/discourse/app/components/topic-timeline.hbs @@ -1,11 +1,3 @@ - -
+
} diff --git a/app/assets/javascripts/discourse/app/components/user-tip.gjs b/app/assets/javascripts/discourse/app/components/user-tip.gjs index 449f6b5e0ce..16d1041e0cf 100644 --- a/app/assets/javascripts/discourse/app/components/user-tip.gjs +++ b/app/assets/javascripts/discourse/app/components/user-tip.gjs @@ -55,14 +55,19 @@ export default class UserTip extends Component { contentText: this.args.contentText ? escape(this.args.contentText) : null, - onDismiss: () => { - this.userTips.hideUserTipForever(this.args.id); - }, buttonText, + buttonSkipText: I18n.t("user_tips.skip"), + showSkipButton: this.args.showSkipButton, }, }); this.tooltip.show(instance); + + if (this.shouldRenderTip) { + // mark tooltip directly as seen so that + // refreshing, clicking outside, etc. won't show it again + this.userTips.markAsSeen(this.args.id); + } }); return () => { diff --git a/app/assets/javascripts/discourse/app/instance-initializers/user-tips.js b/app/assets/javascripts/discourse/app/instance-initializers/user-tips.js deleted file mode 100644 index e1db579ba67..00000000000 --- a/app/assets/javascripts/discourse/app/instance-initializers/user-tips.js +++ /dev/null @@ -1,51 +0,0 @@ -import { bind } from "discourse-common/utils/decorators"; - -export default { - after: "message-bus", - - initialize(owner) { - this.currentUser = owner.lookup("service:current-user"); - if (!this.currentUser) { - return; - } - - this.userTips = owner.lookup("service:user-tips"); - this.messageBus = owner.lookup("service:message-bus"); - this.site = owner.lookup("service:site"); - - this.messageBus.subscribe( - `/user-tips/${this.currentUser.id}`, - this.onMessage - ); - }, - - teardown() { - if (this.currentUser) { - this.messageBus?.unsubscribe( - `/user-tips/${this.currentUser.id}`, - this.onMessage - ); - } - }, - - @bind - onMessage(seenUserTips) { - if (!this.site.user_tips) { - return; - } - - if (!this.currentUser.user_option) { - this.currentUser.set("user_option", {}); - } - - this.currentUser.set("user_option.seen_popups", seenUserTips); - - (seenUserTips || []).forEach((userTipId) => { - this.userTips.hideUserTipForever( - Object.keys(this.site.user_tips).find( - (id) => this.site.user_tips[id] === userTipId - ) - ); - }); - }, -}; diff --git a/app/assets/javascripts/discourse/app/services/user-tips.js b/app/assets/javascripts/discourse/app/services/user-tips.js index 585f8b204e9..569a1ab985d 100644 --- a/app/assets/javascripts/discourse/app/services/user-tips.js +++ b/app/assets/javascripts/discourse/app/services/user-tips.js @@ -39,8 +39,10 @@ export default class UserTips extends Service { } addAvailableTip(tip) { - this.#availableTips.add(tip); - this.#updateRenderedId(); + if (this.canSeeUserTip(tip.id) && !this._findAvailableTipById(tip.id)) { + this.#availableTips.add(tip); + this.#updateRenderedId(); + } } removeAvailableTip(tip) { @@ -84,7 +86,6 @@ export default class UserTips extends Service { return; } - // Empty tipId means all user tips. if (!userTips[tipId]) { // eslint-disable-next-line no-console console.warn("Cannot hide user tip with id", tipId); @@ -94,6 +95,23 @@ export default class UserTips extends Service { const tipObj = [...this.#availableTips].find((t) => t.id === tipId); this.removeAvailableTip(tipObj); + await this.markAsSeen(tipId); + } + + async markAsSeen(tipId) { + if (!this.currentUser) { + return; + } + + if (!tipId) { + return; + } + + const userTips = this.site.user_tips; + if (!userTips || this.currentUser.user_option?.skip_new_user_tips) { + return; + } + // Update list of seen user tips. let seenUserTips = this.currentUser.user_option?.seen_popups || []; if (seenUserTips.includes(userTips[tipId])) { @@ -108,4 +126,25 @@ export default class UserTips extends Service { this.currentUser.set("user_option.seen_popups", seenUserTips); await this.currentUser.save(["seen_popups"]); } + + async skipTips() { + if (!this.currentUser) { + return; + } + + this.#availableTips.clear(); + this.#shouldRenderSet.clear(); + + this.currentUser.set("user_option.skip_new_user_tips", true); + await this.currentUser.save(["skip_new_user_tips"]); + } + + _findAvailableTipById(id) { + for (let item of this.#availableTips) { + if (item.id === id) { + return item; + } + } + return null; + } } diff --git a/app/assets/javascripts/discourse/app/widgets/header-user-tip-shim.js b/app/assets/javascripts/discourse/app/widgets/header-user-tip-shim.js index 32c73fb5bf7..949350e5d27 100644 --- a/app/assets/javascripts/discourse/app/widgets/header-user-tip-shim.js +++ b/app/assets/javascripts/discourse/app/widgets/header-user-tip-shim.js @@ -4,5 +4,5 @@ import { registerWidgetShim } from "discourse/widgets/render-glimmer"; registerWidgetShim( "header-user-tip-shim", "div.header-user-tip-shim", - hbs`` + hbs`` ); diff --git a/app/assets/javascripts/discourse/app/widgets/post-menu.js b/app/assets/javascripts/discourse/app/widgets/post-menu.js index b0d216d1570..031029b0960 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/post-menu.js @@ -621,6 +621,7 @@ export default createWidget("post-menu", { visibleButtons = allButtons; } + let hasShowMoreButton = false; // Only show ellipsis if there is more than one button hidden // if there are no more buttons, we are not collapsed if (!state.collapsed || allButtons.length <= visibleButtons.length + 1) { @@ -636,6 +637,7 @@ export default createWidget("post-menu", { icon: "ellipsis-h", }); visibleButtons.splice(visibleButtons.length - 1, 0, showMore); + hasShowMoreButton = true; } Object.values(_extraButtons).forEach((builder) => { @@ -813,7 +815,9 @@ export default createWidget("post-menu", { }) ); } - + if (hasShowMoreButton) { + contents.push(this.attach("post-user-tip-shim")); + } return contents; }, diff --git a/app/assets/javascripts/discourse/app/widgets/post.js b/app/assets/javascripts/discourse/app/widgets/post.js index e88190aea12..36e69bb76b4 100644 --- a/app/assets/javascripts/discourse/app/widgets/post.js +++ b/app/assets/javascripts/discourse/app/widgets/post.js @@ -1009,10 +1009,7 @@ export default createWidget("post", { return ""; } - return [ - this.attach("post-user-tip-shim"), - this.attach("post-article", attrs), - ]; + return [this.attach("post-article", attrs)]; }, toggleLike() { diff --git a/app/assets/stylesheets/common/base/user-tips.scss b/app/assets/stylesheets/common/base/user-tips.scss index 94fe31ba042..8050fda5e51 100644 --- a/app/assets/stylesheets/common/base/user-tips.scss +++ b/app/assets/stylesheets/common/base/user-tips.scss @@ -29,10 +29,17 @@ .user-tip__buttons { margin-top: 1em; + display: flex; + justify-content: space-between; } .btn-primary { background: var(--secondary); color: var(--tertiary); } + + .btn-flat.btn-text { + background-color: transparent; + color: var(--secondary); + } } diff --git a/app/assets/stylesheets/common/float-kit/d-tooltip.scss b/app/assets/stylesheets/common/float-kit/d-tooltip.scss index c11c451bb2f..c4962b1231c 100644 --- a/app/assets/stylesheets/common/float-kit/d-tooltip.scss +++ b/app/assets/stylesheets/common/float-kit/d-tooltip.scss @@ -64,28 +64,34 @@ &[data-placement^="top"] { .arrow { - bottom: -9px; + bottom: -10px; rotate: 180deg; } } + &[data-placement^="top-start"] { + .arrow { + margin-left: 10px; + } + } + &[data-placement^="bottom"] { .arrow { - top: -9px; + top: -10px; } } &[data-placement^="right"] { .arrow { rotate: -90deg; - left: -9px; + left: -10px; } } &[data-placement^="left"] { .arrow { rotate: 90deg; - right: -9px; + right: -10px; } } } diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 2ff4501bd48..5f7f6272732 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -265,13 +265,6 @@ class UserUpdater user_notification_schedule.destroy_scheduled_timings end end - if attributes.key?(:seen_popups) || attributes.key?(:skip_new_user_tips) - MessageBus.publish( - "/user-tips/#{user.id}", - user.user_option.seen_popups, - user_ids: [user.id], - ) - end DiscourseEvent.trigger(:user_updated, user) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 46390edb754..4e35d1b412f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1990,6 +1990,7 @@ en: user_tips: button: "Got it!" + skip: "Skip tips" first_notification: title: "Your first notification!" diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 93e8ef3614e..65c8a395147 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -586,14 +586,10 @@ RSpec.describe UserUpdater do context "when skip_new_user_tips is edited" do it "updates seen_popups too" do - messages = - MessageBus.track_publish("/user-tips/#{user.id}") do - UserUpdater.new(Discourse.system_user, user).update(skip_new_user_tips: true) - end + UserUpdater.new(Discourse.system_user, user).update(skip_new_user_tips: true) expect(user.user_option.skip_new_user_tips).to eq(true) expect(user.user_option.seen_popups).to eq([-1]) - expect(messages.map(&:data)).to contain_exactly([-1]) end it "does not reset seen_popups" do @@ -606,18 +602,6 @@ RSpec.describe UserUpdater do end end - context "when seen_popups is edited" do - it "publishes a message" do - messages = - MessageBus.track_publish("/user-tips/#{user.id}") do - UserUpdater.new(Discourse.system_user, user).update(seen_popups: [1]) - end - - expect(user.user_option.seen_popups).to eq([1]) - expect(messages.map(&:data)).to contain_exactly([1]) - end - end - it "logs the action" do user = Fabricate(:user, name: "Billy Bob") diff --git a/spec/system/user_tips_spec.rb b/spec/system/user_tips_spec.rb new file mode 100644 index 00000000000..3a3bf6b4ffd --- /dev/null +++ b/spec/system/user_tips_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +describe "Homepage", type: :system do + fab!(:admin) + fab!(:user) + fab!(:topics) { Fabricate.times(2, :post).map(&:topic) } + fab!(:posts) { Fabricate.times(3, :post, topic: topics[0]) } + let(:discovery) { PageObjects::Pages::Discovery.new } + + context "when user tips are disabled" do + before { SiteSetting.enable_user_tips = false } + + it "does not show the 'first notification' tip to the user when disabled" do + sign_in user + visit "/" + + expect(page).to have_no_css(".fk-d-tooltip .user-tip__title") + end + + it "does not show the boostrapping tip to an admin user" do + SiteSetting.bootstrap_mode_enabled = true + sign_in admin + visit "/" + + expect(page).to have_no_css(".fk-d-tooltip .user-tip__title") + end + end + + context "when user tips are enabled" do + before { SiteSetting.enable_user_tips = true } + + it "shows the 'first notification' tip to the user when enabled" do + sign_in user + expect(user.user_option.seen_popups).to eq(nil) + + visit "/" + + expect(page).to have_css(".fk-d-tooltip .user-tip__title", text: "Your first notification!") + + # refresh the page + visit "/" + # Tip is marked as seen when it is displayed + expect(user.reload.user_option.seen_popups).to include(User.user_tips[:first_notification]) + expect(page).to have_no_css( + ".fk-d-tooltip .user-tip__title", + text: "Your first notification!", + ) + end + + it "shows the boostrapping tip to an admin user" do + SiteSetting.bootstrap_mode_enabled = true + expect(admin.user_option.seen_popups).to eq(nil) + sign_in admin + visit "/" + + expect(page).to have_css(".fk-d-tooltip .user-tip__title", text: "Welcome to your new site!") + end + + it "shows a second notification once first is dismissed and user visits a topic" do + sign_in user + visit "/" + + find(".fk-d-tooltip .user-tip__buttons .btn-primary").click + expect(page).to have_no_css(".fk-d-tooltip .user-tip__title") + + discovery.topic_list.visit_topic(topics[0]) + expect(page).to have_css(".fk-d-tooltip .user-tip__title", text: "Topic timeline") + + find(".fk-d-tooltip .user-tip__buttons .btn-primary").click + expect(page).to have_css(".fk-d-tooltip .user-tip__title", text: "Keep reading!") + end + + it "can skip all tips" do + sign_in user + visit "/" + + find(".fk-d-tooltip .user-tip__buttons .btn", text: "Skip tips").click + expect(page).to have_no_css(".fk-d-tooltip .user-tip__title") + + discovery.topic_list.visit_topic(topics[0]) + expect(page).to have_no_css(".fk-d-tooltip .user-tip__title") + end + end +end