From 7611fec0daa6f4f736f1e6e61c8d1dbbf4944179 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 12 Oct 2022 18:38:45 +0300 Subject: [PATCH] FEATURE: Implement new onboarding popups (#18362) This commit introduces a new framework for building user tutorials as popups using the Tippy JS library. Currently, the new framework is used to replace the old notification spotlight and tips and show a new one related to the topic timeline. All popups follow the same structure and have a title, a description and two buttons for either dismissing just the current tip or all of them at once. The state of all seen popups is stored in a user option. Updating skip_new_user_tips will automatically update the list of seen popups accordingly. --- .../discourse/app/components/mount-widget.js | 1 + .../discourse/app/components/site-header.js | 66 +++++++------ .../javascripts/discourse/app/lib/popup.js | 99 +++++++++++++++++++ .../javascripts/discourse/app/models/user.js | 63 +++++++++++- .../discourse/app/widgets/header.js | 45 ++++++++- .../discourse/app/widgets/topic-timeline.js | 26 +++++ .../tests/acceptance/composer-actions-test.js | 1 + .../tests/acceptance/composer-test.js | 1 + .../stylesheets/common/base/discourse.scss | 19 ++++ app/controllers/users_controller.rb | 2 +- app/models/onboarding_popup.rb | 10 ++ app/models/user_option.rb | 1 + app/serializers/current_user_serializer.rb | 9 ++ app/serializers/site_serializer.rb | 9 ++ app/services/user_updater.rb | 9 ++ config/locales/client.en.yml | 12 +++ config/locales/server.en.yml | 1 + config/site_settings.yml | 3 + ...3212549_add_seen_popups_to_user_options.rb | 7 ++ spec/serializers/site_serializer_spec.rb | 14 +++ spec/services/user_updater_spec.rb | 14 +++ 21 files changed, 376 insertions(+), 36 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/popup.js create mode 100644 app/models/onboarding_popup.rb create mode 100644 db/migrate/20220923212549_add_seen_popups_to_user_options.rb diff --git a/app/assets/javascripts/discourse/app/components/mount-widget.js b/app/assets/javascripts/discourse/app/components/mount-widget.js index e520e83a974..6a1538ad5b5 100644 --- a/app/assets/javascripts/discourse/app/components/mount-widget.js +++ b/app/assets/javascripts/discourse/app/components/mount-widget.js @@ -71,6 +71,7 @@ export default Component.extend({ this._connected.forEach((v) => v.destroy()); this._connected.length = 0; + traverseCustomWidgets(this._tree, (w) => w.destroy()); this._rootNode = patch(this._rootNode, diff(this._tree, null)); this._tree = null; }, diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index e44bb9643db..0b902d25639 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -251,39 +251,41 @@ const SiteHeaderComponent = MountWidget.extend( this.currentUser.on("status-changed", this, "queueRerender"); } - if ( - this.currentUser && - !this.get("currentUser.read_first_notification") - ) { - document.body.classList.add("unread-first-notification"); - } + if (!this.siteSettings.enable_onboarding_popups) { + if ( + this.currentUser && + !this.get("currentUser.read_first_notification") + ) { + document.body.classList.add("unread-first-notification"); + } - // Allow first notification to be dismissed on a click anywhere - if ( - this.currentUser && - !this.get("currentUser.read_first_notification") && - !this.get("currentUser.enforcedSecondFactor") - ) { - this._dismissFirstNotification = (e) => { - if (document.body.classList.contains("unread-first-notification")) { - document.body.classList.remove("unread-first-notification"); - } - if ( - !e.target.closest("#current-user") && - !e.target.closest(".ring-backdrop") && - this.currentUser && - !this.get("currentUser.read_first_notification") && - !this.get("currentUser.enforcedSecondFactor") - ) { - this.eventDispatched( - "header:dismiss-first-notification-mask", - "header" - ); - } - }; - document.addEventListener("click", this._dismissFirstNotification, { - once: true, - }); + // Allow first notification to be dismissed on a click anywhere + if ( + this.currentUser && + !this.get("currentUser.read_first_notification") && + !this.get("currentUser.enforcedSecondFactor") + ) { + this._dismissFirstNotification = (e) => { + if (document.body.classList.contains("unread-first-notification")) { + document.body.classList.remove("unread-first-notification"); + } + if ( + !e.target.closest("#current-user") && + !e.target.closest(".ring-backdrop") && + this.currentUser && + !this.get("currentUser.read_first_notification") && + !this.get("currentUser.enforcedSecondFactor") + ) { + this.eventDispatched( + "header:dismiss-first-notification-mask", + "header" + ); + } + }; + document.addEventListener("click", this._dismissFirstNotification, { + once: true, + }); + } } const header = document.querySelector("header.d-header"); diff --git a/app/assets/javascripts/discourse/app/lib/popup.js b/app/assets/javascripts/discourse/app/lib/popup.js new file mode 100644 index 00000000000..f098f75e817 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/popup.js @@ -0,0 +1,99 @@ +import { iconHTML } from "discourse-common/lib/icon-library"; +import I18n from "I18n"; +import { escape } from "pretty-text/sanitizer"; +import tippy from "tippy.js"; + +const instances = {}; +const queue = []; + +export function showPopup(options) { + hidePopup(options.id); + + if (!options.reference) { + return; + } + + if (Object.keys(instances).length > 0) { + return addToQueue(options); + } + + instances[options.id] = tippy(options.reference, { + // Tippy must be displayed as soon as possible and not be hidden unless + // the user clicks on one of the two buttons. + showOnCreate: true, + hideOnClick: false, + trigger: "manual", + + // It must be interactive to make buttons work. + interactive: true, + + arrow: iconHTML("tippy-rounded-arrow"), + placement: options.placement, + + // It often happens for the reference element to be rerendered. In this + // case, tippy must be rerendered too. Having an animation means that the + // animation will replay over and over again. + animation: false, + + // The `content` property below is HTML. + allowHTML: true, + + content: ` +
+
${escape(options.titleText)}
+
${escape( + options.contentText + )}
+
+ + +
+
`, + + onCreate(instance) { + instance.popper + .querySelector(".btn-dismiss") + .addEventListener("click", (event) => { + options.onDismiss(); + event.preventDefault(); + }); + + instance.popper + .querySelector(".btn-dismiss-all") + .addEventListener("click", (event) => { + options.onDismissAll(); + event.preventDefault(); + }); + }, + }); +} + +export function hidePopup(popupId) { + const instance = instances[popupId]; + if (instance && !instance.state.isDestroyed) { + instance.destroy(); + } + delete instances[popupId]; +} + +function addToQueue(options) { + for (let i = 0; i < queue.size; ++i) { + if (queue[i].id === options.id) { + queue[i] = options; + return; + } + } + + queue.push(options); +} + +export function showNextPopup() { + const options = queue.shift(); + if (options) { + showPopup(options); + } +} diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 9476296dfcc..d829fb8e18f 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -35,6 +35,7 @@ import Evented from "@ember/object/evented"; import { cancel } from "@ember/runloop"; import discourseLater from "discourse-common/lib/later"; import { isTesting } from "discourse-common/config/environment"; +import { hidePopup, showNextPopup, showPopup } from "discourse/lib/popup"; export const SECOND_FACTOR_METHODS = { TOTP: 1, @@ -104,6 +105,7 @@ let userOptionFields = [ "title_count_mode", "timezone", "skip_new_user_tips", + "seen_popups", "default_calendar", "bookmark_auto_delete_preference", ]; @@ -441,7 +443,7 @@ const User = RestModel.extend({ "external_links_in_new_tab", "dynamic_favicon" ); - User.current().setProperties(userProps); + User.current()?.setProperties(userProps); this.setProperties(updatedState); return result; }) @@ -1091,6 +1093,65 @@ const User = RestModel.extend({ ) ); }, + + showPopup(options) { + const popupTypes = Site.currentProp("onboarding_popup_types"); + if (!popupTypes[options.id]) { + // eslint-disable-next-line no-console + console.warn("Cannot display popup with type =", options.id); + return; + } + + const seenPopups = this.seen_popups || []; + if (seenPopups.includes(popupTypes[options.id])) { + return; + } + + showPopup({ + ...options, + onDismiss: () => this.hidePopupForever(options.id), + onDismissAll: () => this.hidePopupForever(), + }); + }, + + hidePopupForever(popupId) { + // Empty popupId means all popups. + const popupTypes = Site.currentProp("onboarding_popup_types"); + if (popupId && !popupTypes[popupId]) { + // eslint-disable-next-line no-console + console.warn("Cannot hide popup with type =", popupId); + return; + } + + // Hide any shown popups. + let seenPopups = this.seen_popups || []; + if (popupId) { + hidePopup(popupId); + if (!seenPopups.includes(popupTypes[popupId])) { + seenPopups.push(popupTypes[popupId]); + } + } else { + Object.keys(popupTypes).forEach(hidePopup); + seenPopups = Object.values(popupTypes); + } + + // Show next popup in queue. + showNextPopup(); + + // Save seen popups on the server. + if (!this.user_option) { + this.set("user_option", {}); + } + this.set("seen_popups", seenPopups); + this.set("user_option.seen_popups", seenPopups); + if (popupId) { + return this.save(["seen_popups"]); + } else { + this.set("skip_new_user_tips", true); + this.set("user_option.skip_new_user_tips", true); + return this.save(["seen_popups", "skip_new_user_tips"]); + } + }, }); User.reopenClass(Singleton, { diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index 0a8398f1417..79c8195dd65 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -13,6 +13,7 @@ import { wantsNewWindow } from "discourse/lib/intercept-click"; import { logSearchLinkClick } from "discourse/lib/search"; import RenderGlimmer from "discourse/widgets/render-glimmer"; import { hbs } from "ember-cli-htmlbars"; +import { hidePopup } from "discourse/lib/popup"; let _extraHeaderIcons = []; @@ -87,8 +88,13 @@ createWidget("header-notifications", { const count = unread + reviewables; if (count > 0) { if (this._shouldHighlightAvatar()) { - this._addAvatarHighlight(contents); + if (this.siteSettings.enable_onboarding_popups) { + contents.push(h("span.ring")); + } else { + this._addAvatarHighlight(contents); + } } + contents.push( this.attach("link", { action: attrs.action, @@ -118,7 +124,11 @@ createWidget("header-notifications", { const unreadHighPriority = user.unread_high_priority_notifications; if (!!unreadHighPriority) { if (this._shouldHighlightAvatar()) { - this._addAvatarHighlight(contents); + if (this.siteSettings.enable_onboarding_popups) { + contents.push(h("span.ring")); + } else { + this._addAvatarHighlight(contents); + } } // add the counter for the unread high priority @@ -184,6 +194,37 @@ createWidget("header-notifications", { ) ); }, + + didRenderWidget() { + if ( + !this.currentUser || + !this.siteSettings.enable_onboarding_popups || + !this._shouldHighlightAvatar() + ) { + return; + } + + this.currentUser.showPopup({ + id: "first_notification", + + titleText: I18n.t("popup.first_notification.title"), + contentText: I18n.t("popup.first_notification.content"), + + reference: document + .querySelector(".badge-notification") + ?.parentElement?.querySelector(".avatar"), + + placement: "bottom-end", + }); + }, + + destroy() { + hidePopup("first_notification"); + }, + + willRerenderWidget() { + hidePopup("first_notification"); + }, }); createWidget( diff --git a/app/assets/javascripts/discourse/app/widgets/topic-timeline.js b/app/assets/javascripts/discourse/app/widgets/topic-timeline.js index 34b7358777a..3326b683c53 100644 --- a/app/assets/javascripts/discourse/app/widgets/topic-timeline.js +++ b/app/assets/javascripts/discourse/app/widgets/topic-timeline.js @@ -9,6 +9,7 @@ import discourseLater from "discourse-common/lib/later"; import { relativeAge } from "discourse/lib/formatter"; import renderTags from "discourse/lib/render-tags"; import renderTopicFeaturedLink from "discourse/lib/render-topic-featured-link"; +import { hidePopup } from "discourse/lib/popup"; const SCROLLER_HEIGHT = 50; const LAST_READ_HEIGHT = 20; @@ -598,4 +599,29 @@ export default createWidget("topic-timeline", { return result; }, + + didRenderWidget() { + if (!this.currentUser || !this.siteSettings.enable_onboarding_popups) { + return; + } + + this.currentUser.showPopup({ + id: "topic_timeline", + + titleText: I18n.t("popup.topic_timeline.title"), + contentText: I18n.t("popup.topic_timeline.content"), + + reference: document.querySelector("div.timeline-scrollarea-wrapper"), + + placement: "left", + }); + }, + + destroy() { + hidePopup("topic_timeline"); + }, + + willRerenderWidget() { + hidePopup("topic_timeline"); + }, }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js index 8be69c4bd6c..2f1359d4d31 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js @@ -29,6 +29,7 @@ acceptance("Composer Actions", function (needs) { }); needs.site({ can_tag_topics: true }); needs.pretender((server, helper) => { + server.put("/u/kris.json", () => helper.response({ user: {} })); const cardResponse = cloneJSON(userFixtures["/u/shade/card.json"]); server.get("/u/shade/card.json", () => helper.response(cardResponse)); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js index ab11f39043c..061c086c74c 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js @@ -61,6 +61,7 @@ acceptance("Composer", function (needs) { ], }); needs.pretender((server, helper) => { + server.put("/u/kris.json", () => helper.response({ user: {} })); server.post("/uploads/lookup-urls", () => { return helper.response([]); }); diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss index fe579fe9f0c..9a52a296a8b 100644 --- a/app/assets/stylesheets/common/base/discourse.scss +++ b/app/assets/stylesheets/common/base/discourse.scss @@ -570,6 +570,25 @@ table { animation-name: ping; } +.onboarding-popup-container { + min-width: 300px; + padding: 0.5em; + text-align: left; + + .onboarding-popup-title { + font-size: $font-up-2; + font-weight: bold; + } + + .onboarding-popup-content { + margin-top: 0.25em; + } + + .onboarding-popup-buttons { + margin-top: 1em; + } +} + .fade { opacity: 0; transition: opacity 0.15s linear; diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a28716270e5..1e24e3d5d5d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1966,7 +1966,7 @@ class UsersController < ApplicationController end result = params - .permit(permitted, theme_ids: []) + .permit(permitted, theme_ids: [], seen_popups: []) .reverse_merge( ip_address: request.remote_ip, registration_ip_address: request.remote_ip diff --git a/app/models/onboarding_popup.rb b/app/models/onboarding_popup.rb new file mode 100644 index 00000000000..f672f6eea45 --- /dev/null +++ b/app/models/onboarding_popup.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class OnboardingPopup + def self.types + @types ||= Enum.new( + first_notification: 1, + topic_timeline: 2, + ) + end +end diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 1752ae4fbc3..ac724dbfe66 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -268,6 +268,7 @@ end # oldest_search_log_date :datetime # bookmark_auto_delete_preference :integer default(3), not null # enable_experimental_sidebar :boolean default(FALSE) +# seen_popups :integer is an Array # # Indexes # diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index 0d37b055b40..a144b560e3b 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -68,6 +68,7 @@ class CurrentUserSerializer < BasicUserSerializer :timezone, :featured_topic, :skip_new_user_tips, + :seen_popups, :do_not_disturb_until, :has_topic_draft, :can_review, @@ -282,6 +283,14 @@ class CurrentUserSerializer < BasicUserSerializer object.user_option.skip_new_user_tips end + def seen_popups + object.user_option.seen_popups + end + + def include_seen_popups? + SiteSetting.enable_onboarding_popups + end + def include_primary_group_id? object.primary_group_id.present? end diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index c403767ffec..0ac35515c19 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -6,6 +6,7 @@ class SiteSerializer < ApplicationSerializer :default_archetype, :notification_types, :post_types, + :onboarding_popup_types, :trust_levels, :groups, :filters, @@ -103,6 +104,14 @@ class SiteSerializer < ApplicationSerializer Post.types end + def onboarding_popup_types + OnboardingPopup.types + end + + def include_onboarding_popup_types? + SiteSetting.enable_onboarding_popups + end + def filters Discourse.filters.map(&:to_s) end diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 66e24f23fb6..4de4057b4be 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -47,6 +47,7 @@ class UserUpdater :title_count_mode, :timezone, :skip_new_user_tips, + :seen_popups, :default_calendar ] @@ -178,6 +179,14 @@ class UserUpdater end end + if attributes.key?(:skip_new_user_tips) + user.user_option.seen_popups = if user.user_option.skip_new_user_tips + OnboardingPopup.types.values + else + nil + end + end + # automatically disable digests when mailing_list_mode is enabled user.user_option.email_digests = false if user.user_option.mailing_list_mode diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 3d391db95d7..059e4d1f3b9 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1812,6 +1812,18 @@ en: what_are_you_doing: "What are you doing?" remove_status: "Remove status" + popup: + primary: "Got it!" + secondary: "don't show me these tips" + + first_notification: + title: "Your first notification!" + content: "Notifications are used to keep you up to date with what is happening in the community." + + topic_timeline: + title: "Topic timeline" + content: "Scroll quickly through a post using the topic timeline." + loading: "Loading..." errors: prev_page: "while trying to load" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ba19e4c7f35..a3af49bfd11 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2349,6 +2349,7 @@ en: sitemap_page_size: "Number of URLs to include in each sitemap page. Max 50.000" enable_user_status: "(experimental) Allow users to set custom status message (emoji + description)." + enable_onboarding_popups: "(experimental) Enable educational popups that describe key features to users" short_title: "The short title will be used on the user's home screen, launcher, or other places where space may be limited. It should be limited to 12 characters." diff --git a/config/site_settings.yml b/config/site_settings.yml index 8bccaafaf0e..43100b93b6b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -379,6 +379,9 @@ basic: enable_user_status: client: true default: false + enable_onboarding_popups: + client: true + default: false login: invite_only: diff --git a/db/migrate/20220923212549_add_seen_popups_to_user_options.rb b/db/migrate/20220923212549_add_seen_popups_to_user_options.rb new file mode 100644 index 00000000000..782d4cb3ece --- /dev/null +++ b/db/migrate/20220923212549_add_seen_popups_to_user_options.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddSeenPopupsToUserOptions < ActiveRecord::Migration[7.0] + def change + add_column :user_options, :seen_popups, :integer, array: true + end +end diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb index 4decff31b9f..8f367ec7e82 100644 --- a/spec/serializers/site_serializer_spec.rb +++ b/spec/serializers/site_serializer_spec.rb @@ -8,6 +8,20 @@ RSpec.describe SiteSerializer do Site.clear_cache end + describe '#onboarding_popup_types' do + it 'is included if enable_onboarding_popups' do + SiteSetting.enable_onboarding_popups = true + + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + expect(serialized[:onboarding_popup_types]).to eq(OnboardingPopup.types) + end + + it 'is not included if enable_onboarding_popups is disabled' do + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + expect(serialized[:onboarding_popup_types]).to eq(nil) + end + end + it "includes category custom fields only if its preloaded" do category.custom_fields["enable_marketplace"] = true category.save_custom_fields diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 7539b4e4ad0..abcfc5448f4 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -525,6 +525,20 @@ RSpec.describe UserUpdater do end end + context 'when skip_new_user_tips is edited' do + it 'updates all fields' do + 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(OnboardingPopup.types.values) + + UserUpdater.new(Discourse.system_user, user).update(skip_new_user_tips: false) + + expect(user.user_option.skip_new_user_tips).to eq(false) + expect(user.user_option.seen_popups).to eq(nil) + end + end + it "logs the action" do user = Fabricate(:user, name: 'Billy Bob')