From 6d84a8a1b3187101ebf184a42664f2310b2f8638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 11 Mar 2016 17:51:16 +0100 Subject: [PATCH] FIX: don't send out elided message in email notifications UX: improved details tag for elided messages --- config/locales/server.en.yml | 1 + lib/email/receiver.rb | 9 +++-- .../assets/javascripts/details_dialect.js | 2 ++ .../assets/stylesheets/details.scss | 35 +++++++++++++++++++ plugins/discourse-details/plugin.rb | 17 ++++----- spec/components/email/receiver_spec.rb | 4 +-- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e76ba73b4..8494bb3e0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -50,6 +50,7 @@ en: emails: incoming: default_subject: "Incoming email from %{email}" + show_trimmed_content: "Show trimmed content" errors: empty_email_error: "Happens when the raw mail we received was blank." no_message_id_error: "Happens when the mail has no 'Message-Id' header." diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 27d05b954..24eb6f7cf 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -56,9 +56,14 @@ module Email @incoming_email.update_columns(user_id: user.id) body, elided = select_body - body ||= "" - body << "\n\n[details=...]\n#{elided}\n[/details]" if elided.present? + + if elided.present? + body << "\n\n" << "
" << "\n" + body << "···" << "\n" + body << elided << "\n" + body << "
" << "\n" + end raise AutoGeneratedEmailError if is_auto_generated? raise NoBodyDetectedError if body.blank? && !@mail.has_attachments? diff --git a/plugins/discourse-details/assets/javascripts/details_dialect.js b/plugins/discourse-details/assets/javascripts/details_dialect.js index 4c80824d4..858f887a9 100644 --- a/plugins/discourse-details/assets/javascripts/details_dialect.js +++ b/plugins/discourse-details/assets/javascripts/details_dialect.js @@ -23,4 +23,6 @@ return text; }); + Discourse.Markdown.whiteListTag("details", "class", "elided"); + })(); diff --git a/plugins/discourse-details/assets/stylesheets/details.scss b/plugins/discourse-details/assets/stylesheets/details.scss index f1de007d2..b18333095 100644 --- a/plugins/discourse-details/assets/stylesheets/details.scss +++ b/plugins/discourse-details/assets/stylesheets/details.scss @@ -7,6 +7,11 @@ details .lightbox-wrapper { display: none; } +details, +summary { + outline: none; +} + summary:first-of-type { cursor: pointer; display: block; @@ -36,3 +41,33 @@ summary::-webkit-details-marker { details .lazyYT-container { display: none; } + + +.elided { + color: dark-light-choose(scale-color($primary, $lightness: 65%), scale-color($secondary, $lightness: 35%)); + + summary:before { + content: '' !important; + } + + summary { + @include unselectable; + box-sizing: border-box; + margin: 0; + padding: 0; + color: #aaa; + background: #f1f1f1; + border: 1px solid #ddd; + width: 20px; + display: flex; + text-align: center; + vertical-align: middle; + line-height: 12px; + } + + summary:hover { + color: #222; + background: #d8d8d8; + border-color: #cdcdcd; + } +} diff --git a/plugins/discourse-details/plugin.rb b/plugins/discourse-details/plugin.rb index c028cf305..41a1a61a7 100644 --- a/plugins/discourse-details/plugin.rb +++ b/plugins/discourse-details/plugin.rb @@ -1,6 +1,6 @@ # name: discourse-details # about: HTML5.1 Details polyfill for Discourse -# version: 0.3 +# version: 0.4 # authors: Régis Hanol # url: https://github.com/discourse/discourse/tree/master/plugins/discourse-details @@ -13,14 +13,15 @@ register_asset "stylesheets/details.scss" after_initialize do - # replace all details with their summary in emails Email::Styles.register_plugin_style do |fragment| - if SiteSetting.details_enabled - fragment.css("details").each do |details| - summary = details.css("summary")[0] - summary.name = "p" - details.replace(summary) - end + # remove all elided content + fragment.css("details.elided").each { |d| d.remove } + + # replace all details with their summary in emails + fragment.css("details").each do |details| + summary = details.css("summary")[0] + summary.name = "p" + details.replace(summary) end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 164d60cb4..697231e43 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -118,7 +118,7 @@ describe Email::Receiver do it "removes the 'on , wrote' quoting line" do expect { process(:on_date_contact_wrote) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to eq("This is the actual reply.\n\n[details=...]\nOn Tue, Jan 14, 2016 at 0:42 AM, Bar Foo wrote:\n\n> This is the previous email.\n> And it had\n>\n> a lot\n>\n>\n> of lines ;)\n[/details]") + expect(topic.posts.last.raw).to eq("This is the actual reply.\n\n
\n···\nOn Tue, Jan 14, 2016 at 0:42 AM, Bar Foo wrote:\n\n> This is the previous email.\n> And it had\n>\n> a lot\n>\n>\n> of lines ;)\n
") end it "removes the 'Previous Replies' marker" do @@ -193,7 +193,7 @@ describe Email::Receiver do it "strips 'original message' context" do expect { process(:original_message) }.to change { topic.posts.count } - expect(topic.posts.last.raw).to eq("This is a reply :)\n\n[details=...]\n---Original Message---\nThis part should not be included\n[/details]") + expect(topic.posts.last.raw).to eq("This is a reply :)\n\n
\n···\n---Original Message---\nThis part should not be included\n
") end it "supports attached images" do