From ae1d2d957f21d8c959e0aa904348f7251e3748bd Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 21 Feb 2022 22:14:28 +0200 Subject: [PATCH] FEATURE: Replace share post popup with share modal (#15875) This uniformizes the topic share modal and the post link popup. It also introduces a new feature which can notify the user of a post. --- .../discourse/app/controllers/share-topic.js | 27 ++++++++++++++----- .../app/initializers/topic-footer-buttons.js | 2 +- .../app/templates/modal/share-topic.hbs | 8 +++--- .../discourse/app/templates/topic.hbs | 1 - .../discourse/app/widgets/post-menu.js | 6 +---- .../javascripts/discourse/app/widgets/post.js | 11 ++++++++ .../tests/integration/widgets/post-test.js | 5 +--- app/controllers/topics_controller.rb | 13 +++++++-- app/models/topic.rb | 4 +-- config/locales/client.en.yml | 6 ++++- spec/requests/topics_controller_spec.rb | 4 +++ 11 files changed, 62 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/share-topic.js b/app/assets/javascripts/discourse/app/controllers/share-topic.js index b9a06513730..638a704c319 100644 --- a/app/assets/javascripts/discourse/app/controllers/share-topic.js +++ b/app/assets/javascripts/discourse/app/controllers/share-topic.js @@ -16,19 +16,31 @@ export default Controller.extend( bufferedProperty("invite"), { topic: null, + post: null, + allowInvites: false, + showNotifyUsers: false, restrictedGroups: null, onShow() { - this.set("showNotifyUsers", false); + this.setProperties({ + topic: null, + post: null, + allowInvites: false, + showNotifyUsers: false, + }); if (this.model && this.model.read_restricted) { this.restrictedGroupWarning(); } }, - @discourseComputed("topic.shareUrl") - topicUrl(url) { - return url ? getAbsoluteURL(url) : null; + @discourseComputed("post.shareUrl", "topic.shareUrl") + url(postUrl, topicUrl) { + if (postUrl) { + return getAbsoluteURL(postUrl); + } else if (topicUrl) { + return getAbsoluteURL(topicUrl); + } }, @discourseComputed( @@ -57,7 +69,7 @@ export default Controller.extend( this.set("showNotifyUsers", false); Sharing.shareSource(source, { title: this.topic.title, - url: this.topicUrl, + url: this.url, }); }, @@ -81,7 +93,10 @@ export default Controller.extend( ajax(`/t/${this.topic.id}/invite-notify`, { type: "POST", - data: { usernames: this.users }, + data: { + usernames: this.users, + post_number: this.post ? this.post.post_number : undefined, + }, }) .then(() => { this.setProperties({ showNotifyUsers: false }); diff --git a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js index a5a52277718..e00c100309f 100644 --- a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js +++ b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js @@ -18,7 +18,7 @@ export default { priority: SHARE_PRIORITY, label() { if (!this.get("topic.isPrivateMessage") || this.site.mobileView) { - return "topic.share.title"; + return "footer_nav.share"; } }, title: "topic.share.help", diff --git a/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs b/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs index 330637850b8..8c4c3a7a857 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs @@ -1,12 +1,14 @@ -{{#d-modal-body title="topic.share.title"}} +{{#d-modal-body rawTitle=(if post (i18n "post.share.title" post_number=post.post_number) (i18n "topic.share.title"))}}
- +
{{input name="invite-link" class="invite-link" - value=topicUrl + value=url readonly=true }} {{copy-button selector="input.invite-link"}} diff --git a/app/assets/javascripts/discourse/app/templates/topic.hbs b/app/assets/javascripts/discourse/app/templates/topic.hbs index bd82be5733e..e9913db6813 100644 --- a/app/assets/javascripts/discourse/app/templates/topic.hbs +++ b/app/assets/javascripts/discourse/app/templates/topic.hbs @@ -414,6 +414,5 @@ editPost=(action "editPost") topic=model composerVisible=composer.visible - }} {{/discourse-topic}} diff --git a/app/assets/javascripts/discourse/app/widgets/post-menu.js b/app/assets/javascripts/discourse/app/widgets/post-menu.js index 63c76f053a3..a468c69e6b6 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/post-menu.js @@ -271,16 +271,12 @@ registerButton("replies", (attrs, state, siteSettings) => { }; }); -registerButton("share", (attrs) => { +registerButton("share", () => { return { action: "share", className: "share", title: "post.controls.share", icon: "d-post-share", - data: { - "share-url": attrs.shareUrl, - "post-number": attrs.post_number, - }, }; }); diff --git a/app/assets/javascripts/discourse/app/widgets/post.js b/app/assets/javascripts/discourse/app/widgets/post.js index a32d87d7309..f358db869bf 100644 --- a/app/assets/javascripts/discourse/app/widgets/post.js +++ b/app/assets/javascripts/discourse/app/widgets/post.js @@ -21,6 +21,8 @@ import { prioritizeNameInUx } from "discourse/lib/settings"; import { relativeAgeMediumSpan } from "discourse/lib/formatter"; import { transformBasicPost } from "discourse/lib/transform-post"; import autoGroupFlairForUser from "discourse/lib/avatar-flair"; +import showModal from "discourse/lib/show-modal"; +import { nativeShare } from "discourse/lib/pwa-utils"; function transformWithCallbacks(post) { let transformed = transformBasicPost(post); @@ -537,6 +539,15 @@ createWidget("post-contents", { const post = this.findAncestorModel(); return post.expand().then(() => (this.state.expandedFirstPost = true)); }, + + share() { + const post = this.findAncestorModel(); + nativeShare(this.capabilities, { url: post.shareUrl }).catch(() => { + const topic = post.topic; + const controller = showModal("share-topic", { model: topic.category }); + controller.setProperties({ topic, post }); + }); + }, }); createWidget("post-notice", { diff --git a/app/assets/javascripts/discourse/tests/integration/widgets/post-test.js b/app/assets/javascripts/discourse/tests/integration/widgets/post-test.js index 187617dcc91..ea634b3dd79 100644 --- a/app/assets/javascripts/discourse/tests/integration/widgets/post-test.js +++ b/app/assets/javascripts/discourse/tests/integration/widgets/post-test.js @@ -229,10 +229,7 @@ discourseModule("Integration | Component | Widget | post", function (hooks) { this.set("args", { shareUrl: "http://share-me.example.com" }); }, test(assert) { - assert.ok( - exists(".actions button[data-share-url]"), - "it renders a share button" - ); + assert.ok(exists(".actions button.share"), "it renders a share button"); }, }); diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index abb249c2e8d..e1bb02f7993 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -688,6 +688,10 @@ class TopicsController < ApplicationController users = User.where(username_lower: usernames.map(&:downcase)) raise Discourse::InvalidParameters.new(:usernames) if usernames.size != users.size + post_number = 1 + post_number = params[:post_number].to_i if params[:post_number].present? + raise Discourse::InvalidParameters.new(:post_number) if post_number < 1 || post_number > topic.highest_post_number + topic.rate_limit_topic_invitation(current_user) users.find_each do |user| @@ -700,11 +704,16 @@ class TopicsController < ApplicationController last_notification = user.notifications .where(notification_type: Notification.types[:invited_to_topic]) .where(topic_id: topic.id) - .where(post_number: 1) + .where(post_number: post_number) .where('created_at > ?', 1.hour.ago) if !last_notification.exists? - topic.create_invite_notification!(user, Notification.types[:invited_to_topic], current_user.username) + topic.create_invite_notification!( + user, + Notification.types[:invited_to_topic], + current_user.username, + post_number: post_number + ) end end diff --git a/app/models/topic.rb b/app/models/topic.rb index d3dc3047001..bfbc2c1b922 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -1756,14 +1756,14 @@ class Topic < ActiveRecord::Base email_addresses.to_a end - def create_invite_notification!(target_user, notification_type, username) + def create_invite_notification!(target_user, notification_type, username, post_number: 1) invited_by = User.find_by_username(username) ensure_can_invite!(target_user, invited_by) target_user.notifications.create!( notification_type: notification_type, topic_id: self.id, - post_number: 1, + post_number: post_number, data: { topic_title: self.title, display_username: username, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 95ebcc3e3f3..c6796171056 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2840,7 +2840,7 @@ en: help: "begin composing a reply to this topic" share: - title: "Share" + title: "Share Topic" extended_title: "Share a link" help: "share a link to this topic" instructions: "Share a link to this topic:" @@ -3282,6 +3282,10 @@ en: post_number: "%{username}, post #%{post_number}" show_all: "Show all" + share: + title: "Share Post #%{post_number}" + instructions: "Share a link to this post:" + category: none: "(no category)" all: "All categories" diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 4d7396fd305..37d8366a2c5 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2853,6 +2853,10 @@ RSpec.describe TopicsController do end describe '#invite_notify' do + before do + topic.update!(highest_post_number: 1) + end + it 'does not notify same user multiple times' do sign_in(user)