Skip to content

Add help text for proposals' 'publish answers immediately' setting #9549

Merged
ahukkanen merged 5 commits intodevelopfrom
fix/proposals-answer-help
Jul 15, 2022
Merged

Add help text for proposals' 'publish answers immediately' setting #9549
ahukkanen merged 5 commits intodevelopfrom
fix/proposals-answer-help

Conversation

@andreslucena
Copy link
Copy Markdown
Member

🎩 What? Why?

While reviewing the 'publish answers immediately' setting, I had to double-check the history definition to see how it should work. As that's not optimal, I've made a change in the documentation to clarify it decidim/documentation#91. This PR adds the link in the admin to this page.

For this, I needed to add HTML support to the help text feature in the SettingsHelper. Mind that I've only done that in the text types, as that's where I had the need. Should we add it to the other attribute types, or should we apply YAGNI?

As a bonus track, I've also converted the docs of this file to yardoc and also added more detail. I can extract this commit to another PR as it's not relevant, but it shouldn't break anything of course as its comments.

I couldn't find an elegant way to test this. I've seen that we have this spec https://github.com/decidim/decidim/blob/develop/decidim-admin/spec/helpers/settings_helper_spec.rb, but the helper has lots more logic than this spec has. Should we add that there? Or there's another spec(s) that indirectly cover these features?

📌 Related Issues

Testing

  1. Sign in as admin
  2. Go to admin dashboard
  3. Go to a participatory space
  4. Add a new proposal component
  5. See the new help text

📷 Screenshots

image

♥️ Thank you!

@andreslucena andreslucena added module: proposals module: admin type: fix PRs that implement a fix for a bug labels Jul 7, 2022
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, I needed to add HTML support to the help text feature in the SettingsHelper. Mind that I've only done that in the text types, as that's where I had the need. Should we add it to the other attribute types, or should we apply YAGNI?

I believe it already covers all the attribute types as-is. You are by the way adding it for a checkbox, not a text attribute type (?).

I believe this method is called for all attribute types:

help_text ||= text_for_setting(name, "help", i18n_scope)

As a bonus track, I've also converted the docs of this file to yardoc and also added more detail. I can extract this commit to another PR as it's not relevant, but it shouldn't break anything of course as its comments.

No need to extract to another PR as it's not too many changes here, so it's manageable within this PR.

I couldn't find an elegant way to test this. I've seen that we have this spec https://github.com/decidim/decidim/blob/develop/decidim-admin/spec/helpers/settings_helper_spec.rb, but the helper has lots more logic than this spec has. Should we add that there? Or there's another spec(s) that indirectly cover these features?

You will need to define an actual form helper and a render context after which you can test it e.g. as follows in the related spec:

      describe "help texts" do
        let(:form) { Decidim::Admin::FormBuilder.new(:foo, double(name => value), template, {}) }
        let(:template) { Class.new(ActionView::Base).new(ActionView::LookupContext.new(ActionController::Base.view_paths), {}, []) }
        let(:type) { :boolean }
        let(:name) { :guided }

        it "renders the help text" do
          expect(render_input).to include(%(<span class="help-text">Help text</span>))
        end

        context "with HTML enriched help text" do
          let(:name) { :guided_rich }

          it "renders the HTML formatted help text" do
            expect(render_input).to include(%(<span class="help-text">HTML <strong>help</strong> text</span>))
          end
        end
      end

You will also need to define the following keys for the dummy global settings:

       dummy:
         settings:
           global:
+            guided: Guided input
+            guided_help: Help text
+            guided_rich: Guided rich input
+            guided_rich_help_html: HTML <strong>help</strong> text

Here:

@andreslucena andreslucena force-pushed the fix/proposals-answer-help branch from 2e5671f to 14ca5e1 Compare July 14, 2022 08:25
@andreslucena
Copy link
Copy Markdown
Member Author

I believe it already covers all the attribute types as-is. You are by the way adding it for a checkbox, not a text attribute type (?).

I believe this method is called for all attribute types:

Oh, of course, you're right! I guess my brain just saw "text" in the method name and stopped thinking 😅

You will need to define an actual form helper and a render context after which you can test it e.g. as follows in the related spec:

Awesome! I've applied it already

@ahukkanen ahukkanen merged commit c1dd072 into develop Jul 15, 2022
@ahukkanen ahukkanen deleted the fix/proposals-answer-help branch July 15, 2022 12:02
entantoencuanto added a commit that referenced this pull request Jul 15, 2022
…ging

* feature/redesign-main-footer:
  Reorder elements in main links of footer and define links and texts
  Define a cell for static_pages and topics configured to appear in footer
  Fix translation call
  Set fixed links in redesigned_main_legal partial
  Add FooterMenuPresenter to display menu items in footer
  Fix budgets seeds on non development apps (#9585)
  Return 404 when there isn't a valid component in program (#9576)
  Add missing queue close_meeting_reminder to sidekiq configuration (#9568)
  Make the HERE Map display in the currently selected language (#9552)
  Add help text for proposals' 'publish answers immediately' setting  (#9549)
  Fix admin language selector with more than 4 locales (#9519)
  Fix publish event on official proposals (#9421)
  Prevent missing ActionLog entries to break the application (#9502)
  Add boilerplate structure to CHANGELOG (#9501)
  Add step-by-step instructions of the Crowdin releases process (#9555)
  Fix translated attributes field type change (#9547)
  Add `modifyList` option to the autocomplete element (#9548)
  Admin log filters (#9460)
  Improve the default gitignore files created by the generators (#9507)
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
…ecidim#9549)

* Add support for HTML on settings' help

* Add help text for proposals' 'publish answers immediately' setting

* Add documentation to SettingsHelper

* Add spec for help texts feature

* Add spec for #text_for_setting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: admin module: proposals type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants