From babbebfb3571b09bfe6600bbe0921ded3c29fb3e Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Fri, 2 Oct 2020 09:21:24 +0200 Subject: [PATCH] FEATURE: Add the title attribute to polls (#10759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an optional title attribute to polls. The rationale for this addition is that polls themselves didn't contain context/question and relied on post body to explain them. That context wasn't always obvious (e.g. when there are multiple polls in a single post) or available (e.g. when you display the poll breakdown - you see the answers, but not the question) As a side note, here's a word on how the poll plugin works: > We have a markdown poll renderer, which we use in the builder UI and the composer preview, but… when you submit a post, raw markdown is cooked into html (twice), then we extract data from the generated html and save it to the database. When it's render time, we first display the cooked html poll, and then extract some data from that html, get the data from the post's JSON (and identify that poll using the extracted html stuff) to then render the poll using widgets and the JSON data. --- plugins/poll/app/models/poll.rb | 1 + .../poll/app/serializers/poll_serializer.rb | 3 +- .../controllers/poll-breakdown.js.es6 | 6 ++++ .../controllers/poll-ui-builder.js.es6 | 8 +++++ .../templates/modal/poll-breakdown.hbs | 5 +-- .../templates/modal/poll-ui-builder.hbs | 5 +++ .../initializers/extend-for-poll.js.es6 | 3 ++ .../lib/discourse-markdown/poll.js.es6 | 32 ++++++++++++++++++- .../javascripts/widgets/discourse-poll.js.es6 | 4 +++ .../stylesheets/common/poll-ui-builder.scss | 13 ++++++-- .../poll/assets/stylesheets/common/poll.scss | 8 ++--- .../poll/assets/stylesheets/desktop/poll.scss | 5 +++ plugins/poll/config/locales/client.en.yml | 2 ++ .../20200804144550_add_title_to_polls.rb | 7 ++++ plugins/poll/lib/polls_updater.rb | 2 +- plugins/poll/plugin.rb | 7 ++++ .../spec/controllers/posts_controller_spec.rb | 11 +++++++ plugins/poll/spec/lib/pretty_text_spec.rb | 20 ++++++++++-- 18 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 plugins/poll/db/migrate/20200804144550_add_title_to_polls.rb diff --git a/plugins/poll/app/models/poll.rb b/plugins/poll/app/models/poll.rb index 210533836f..cbb3016f46 100644 --- a/plugins/poll/app/models/poll.rb +++ b/plugins/poll/app/models/poll.rb @@ -83,6 +83,7 @@ end # updated_at :datetime not null # chart_type :integer default("bar"), not null # groups :string +# title :string # # Indexes # diff --git a/plugins/poll/app/serializers/poll_serializer.rb b/plugins/poll/app/serializers/poll_serializer.rb index 171eb95387..6444349bac 100644 --- a/plugins/poll/app/serializers/poll_serializer.rb +++ b/plugins/poll/app/serializers/poll_serializer.rb @@ -14,7 +14,8 @@ class PollSerializer < ApplicationSerializer :close, :preloaded_voters, :chart_type, - :groups + :groups, + :title def public true diff --git a/plugins/poll/assets/javascripts/controllers/poll-breakdown.js.es6 b/plugins/poll/assets/javascripts/controllers/poll-breakdown.js.es6 index 6a47eeeaa9..89effd9272 100644 --- a/plugins/poll/assets/javascripts/controllers/poll-breakdown.js.es6 +++ b/plugins/poll/assets/javascripts/controllers/poll-breakdown.js.es6 @@ -2,6 +2,7 @@ import I18n from "I18n"; import Controller from "@ember/controller"; import { action } from "@ember/object"; import { classify } from "@ember/string"; +import { htmlSafe } from "@ember/template"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; import loadScript from "discourse/lib/load-script"; @@ -15,6 +16,11 @@ export default Controller.extend(ModalFunctionality, { highlightedOption: null, displayMode: "percentage", + @discourseComputed("model.poll.title", "model.post.topic.title") + title(pollTitle, topicTitle) { + return pollTitle ? htmlSafe(pollTitle) : topicTitle; + }, + @discourseComputed("model.groupableUserFields") groupableUserFields(fields) { return fields.map((field) => { diff --git a/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 b/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 index e5aa341108..9090e92f65 100644 --- a/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 +++ b/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 @@ -28,6 +28,7 @@ export default Controller.extend({ pollType: null, pollResult: null, + pollTitle: null, init() { this._super(...arguments); @@ -214,6 +215,7 @@ export default Controller.extend({ "pollType", "pollResult", "publicPoll", + "pollTitle", "pollOptions", "pollMin", "pollMax", @@ -230,6 +232,7 @@ export default Controller.extend({ pollType, pollResult, publicPoll, + pollTitle, pollOptions, pollMin, pollMax, @@ -293,6 +296,10 @@ export default Controller.extend({ pollHeader += "]"; output += `${pollHeader}\n`; + if (pollTitle) { + output += `# ${pollTitle.trim()}\n`; + } + if (pollOptions.length > 0 && !isNumber) { pollOptions.split("\n").forEach((option) => { if (option.length !== 0) { @@ -382,6 +389,7 @@ export default Controller.extend({ chartType: BAR_CHART_TYPE, pollResult: this.alwaysPollResult, pollGroups: null, + pollTitle: null, date: moment().add(1, "day").format("YYYY-MM-DD"), time: moment().add(1, "hour").format("HH:mm"), }); diff --git a/plugins/poll/assets/javascripts/discourse/templates/modal/poll-breakdown.hbs b/plugins/poll/assets/javascripts/discourse/templates/modal/poll-breakdown.hbs index 0cb26c08d5..693e31a34e 100644 --- a/plugins/poll/assets/javascripts/discourse/templates/modal/poll-breakdown.hbs +++ b/plugins/poll/assets/javascripts/discourse/templates/modal/poll-breakdown.hbs @@ -1,7 +1,8 @@ {{#d-modal-body title="poll.breakdown.title"}}
- {{!-- TODO: replace with the (optional) poll title --}} -

{{this.model.post.topic.title}}

+

+ {{this.title}} +

{{i18n "poll.breakdown.votes" count=this.model.poll.voters}}
diff --git a/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs b/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs index 03e08acee7..3679c83037 100644 --- a/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs +++ b/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs @@ -77,6 +77,11 @@ {{/if}} {{/if}} +
+ + {{input value=pollTitle}} +
+ {{#unless isNumber}}
diff --git a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 index d8a2262b80..46ec6e86c4 100644 --- a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 +++ b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 @@ -88,11 +88,14 @@ function initializePolls(api) { } if (poll) { + const titleElement = pollElem.querySelector(".poll-title"); + const attrs = { id: `${pollName}-${pollPost.id}`, post: pollPost, poll, vote, + titleHTML: titleElement && titleElement.outerHTML, groupableUserFields: ( api.container.lookup("site-settings:main") .poll_groupable_user_fields || "" diff --git a/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 b/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 index a6b8222910..b098a0b65b 100644 --- a/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 +++ b/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 @@ -81,6 +81,22 @@ function invalidPoll(state, tag) { token.content = "[/" + tag + "]"; } +function getTitle(tokens) { + const open = tokens.findIndex((token) => token.type === "heading_open"); + const close = tokens.findIndex((token) => token.type === "heading_close"); + + if (open === -1 || close === -1) { + return; + } + + const titleTokens = tokens.slice(open + 1, close); + + // Remove the heading element + tokens.splice(open, close - open + 1); + + return titleTokens; +} + const rule = { tag: "poll", @@ -92,7 +108,9 @@ const rule = { }, after: function (state, openToken, raw) { + const titleTokens = getTitle(state.tokens); let items = getListItems(state.tokens, openToken); + if (!items) { return invalidPoll(state, raw); } @@ -139,9 +157,19 @@ const rule = { token = new state.Token("poll_open", "div", 1); token.attrs = [["class", "poll-container"]]; - header.push(token); + if (titleTokens) { + token = new state.Token("title_open", "div", 1); + token.attrs = [["class", "poll-title"]]; + header.push(token); + + header.push(...titleTokens); + + token = new state.Token("title_close", "div", -1); + header.push(token); + } + // generate the options when the type is "number" if (attrs["type"] === "number") { // default values @@ -175,6 +203,7 @@ const rule = { token = new state.Token("list_item_close", "li", -1); header.push(token); } + token = new state.Token("bullet_item_close", "", -1); header.push(token); } @@ -240,6 +269,7 @@ export function setup(helper) { "div.poll", "div.poll-info", "div.poll-container", + "div.poll-title", "div.poll-buttons", "div[data-*]", "span.info-number", diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 index cc5c2bea5a..241bbb73ea 100644 --- a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 +++ b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 @@ -356,6 +356,8 @@ createWidget("discourse-poll-container", { } else if (options) { const contents = []; + contents.push(new RawHtml({ html: attrs.titleHTML })); + if (!checkUserGroups(this.currentUser, poll)) { contents.push( h( @@ -511,6 +513,8 @@ createWidget("discourse-poll-pie-chart", { contents.push(button); } + contents.push(new RawHtml({ html: attrs.titleHTML })); + const chart = this.attach("discourse-poll-pie-canvas", attrs); contents.push(chart); diff --git a/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss b/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss index 7f38887ed4..a4955b8eec 100644 --- a/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss +++ b/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss @@ -56,17 +56,26 @@ $poll-margin: 10px; } } - .poll-textarea { + .poll-textarea, + .poll-title { flex-direction: column; } + .poll-title input { + width: 100%; + } + .poll-textarea textarea { width: 100%; height: 90px; box-sizing: border-box; } - .poll-select + .poll-textarea { + .poll-select + .poll-title { + margin-top: $poll-margin; + } + + .poll-textarea { margin-top: $poll-margin; } diff --git a/plugins/poll/assets/stylesheets/common/poll.scss b/plugins/poll/assets/stylesheets/common/poll.scss index a40bb78917..828a5e12ed 100644 --- a/plugins/poll/assets/stylesheets/common/poll.scss +++ b/plugins/poll/assets/stylesheets/common/poll.scss @@ -149,21 +149,21 @@ div.poll { } .poll-results-chart { - height: 320px; + height: 340px; overflow-y: auto; overflow-x: hidden; } .poll-show-breakdown { - margin-bottom: 10px; + margin-bottom: 0.25em; } } div.poll.pie { .poll-container { display: inline-block; - height: 320px; - max-height: 320px; + height: 340px; + max-height: 340px; overflow-y: auto; } .poll-info { diff --git a/plugins/poll/assets/stylesheets/desktop/poll.scss b/plugins/poll/assets/stylesheets/desktop/poll.scss index bbfffda269..d6e3aa0771 100644 --- a/plugins/poll/assets/stylesheets/desktop/poll.scss +++ b/plugins/poll/assets/stylesheets/desktop/poll.scss @@ -28,6 +28,11 @@ div.poll { border-right: 1px solid var(--primary-low); } + .poll-title { + border-bottom: 1px solid var(--primary-low); + padding: 0.5em 0; + } + .poll-buttons { border-top: 1px solid var(--primary-low); padding: 1em; diff --git a/plugins/poll/config/locales/client.en.yml b/plugins/poll/config/locales/client.en.yml index 856009b504..9b47e0f400 100644 --- a/plugins/poll/config/locales/client.en.yml +++ b/plugins/poll/config/locales/client.en.yml @@ -112,6 +112,8 @@ en: step: Step poll_public: label: Show who voted + poll_title: + label: Title (optional) poll_options: label: Enter one poll option per line automatic_close: diff --git a/plugins/poll/db/migrate/20200804144550_add_title_to_polls.rb b/plugins/poll/db/migrate/20200804144550_add_title_to_polls.rb new file mode 100644 index 0000000000..253f2cc39a --- /dev/null +++ b/plugins/poll/db/migrate/20200804144550_add_title_to_polls.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddTitleToPolls < ActiveRecord::Migration[6.0] + def change + add_column :polls, :title, :string + end +end diff --git a/plugins/poll/lib/polls_updater.rb b/plugins/poll/lib/polls_updater.rb index f50edc0f4e..39b48f45d2 100644 --- a/plugins/poll/lib/polls_updater.rb +++ b/plugins/poll/lib/polls_updater.rb @@ -3,7 +3,7 @@ module DiscoursePoll class PollsUpdater - POLL_ATTRIBUTES ||= %w{close_at max min results status step type visibility groups} + POLL_ATTRIBUTES ||= %w{close_at max min results status step type visibility title groups} def self.update(post, polls) ::Poll.transaction do diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 391610c26f..513a2e494d 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -329,6 +329,7 @@ after_initialize do type: poll["type"].presence || "regular", status: poll["status"].presence || "open", visibility: poll["public"] == "true" ? "everyone" : "secret", + title: poll["title"], results: poll["results"].presence || "always", min: poll["min"], max: poll["max"], @@ -367,6 +368,12 @@ after_initialize do poll["options"] << { "id" => option_id, "html" => o.inner_html.strip } end + # title + title_element = p.css(".poll-title").first + if title_element + poll["title"] = title_element.inner_html.strip + end + poll end end diff --git a/plugins/poll/spec/controllers/posts_controller_spec.rb b/plugins/poll/spec/controllers/posts_controller_spec.rb index be26174871..9024c8d6e9 100644 --- a/plugins/poll/spec/controllers/posts_controller_spec.rb +++ b/plugins/poll/spec/controllers/posts_controller_spec.rb @@ -139,6 +139,17 @@ describe PostsController do expect(Poll.where(post_id: json["id"]).count).to eq(1) end + it "accepts polls with titles" do + post :create, params: { + title: title, raw: "[poll]\n# What's up?\n- one\n[/poll]" + }, format: :json + + expect(response).to be_successful + poll = Poll.last + expect(poll).to_not be_nil + expect(poll.title).to eq("What’s up?") + end + describe "edit window" do describe "within the first 5 minutes" do diff --git a/plugins/poll/spec/lib/pretty_text_spec.rb b/plugins/poll/spec/lib/pretty_text_spec.rb index c4d09d2aac..b76d9c320d 100644 --- a/plugins/poll/spec/lib/pretty_text_spec.rb +++ b/plugins/poll/spec/lib/pretty_text_spec.rb @@ -95,7 +95,7 @@ describe PrettyText do cooked = PrettyText.cook md - expected = <<~MD + expected = <<~HTML
@@ -113,7 +113,7 @@ describe PrettyText do
- MD + HTML # note, hashes should remain stable even if emoji changes cause text content is hashed expect(n cooked).to eq(n expected) @@ -153,4 +153,20 @@ describe PrettyText do excerpt = PrettyText.excerpt(post.cooked, SiteSetting.post_onebox_maxlength) expect(excerpt).to eq("A post with a poll \npoll") end + + it "supports the title attribute" do + cooked = PrettyText.cook <<~MD + [poll] + # What's your favorite *berry*? :wink: https://google.com/ + * Strawberry + * Raspberry + * Blueberry + [/poll] + MD + + expect(cooked).to include(<<~HTML) +
What’s your favorite berry? :wink: https://google.com/ +
+ HTML + end end