Extract 'write a reponse/reply' into it's own button

[wrightmartin]

Relevant issue(s)

Fixes #6363

What does this do?

Moves the oft-used 'write a reply/response' button to it's own prominent button at the bottom of request threads

Why was this needed?

It's reduces the number of clicks needed to access a well-used function. It will also make this function more prominent in the UI and help show what kinds of things are possible on the site

Implementation notes

I've left the action in the drop down menu as it's used at the top of the request thread and in other places. As a result I haven't renamed it 'more actions' as the original ticket suggests

Screenshots

Notes to reviewer

I extracted the reply button to a partial, you may or may not agree with that decision - I'll leave that to the reviewer to approve

[garethrees]

Agree with using a partial here.

Looks good on WDTK, but looks a bit borked elsewhere. Wouldn't expect re-users to need to make theme updates for this – I'm not sure why WDTK handles the change without modification?

There's a separate set of actions for pros, so we'll need to render this there too.

[garethrees]

Also, I think "Reply" should come before "Actions", since it's the most common action to take.

That does make the button colours a bit weird. Really the "follow" button shouldn't be a primary action, but I'll ticket that up separately.

[wrightmartin]

I've pushed those improvements but it looks like removing the buttony style removes the styling for WDTK theme

I'm looking at the pro display now

[wrightmartin]

All feedback addressed and ready to look at again

[garethrees]

Couple of super minor comments inline, but I think the main thing that would be great to address is making this zero-config for re-users. I can see we've made an update to WDTK (mysociety/whatdotheyknow-theme#871), but we're going to have to remember to do this for all themes.

Is there a better way? If the answer is "Not without a load of effort", then I guess we'll have to settle for this. Have you had a look at whether we can include the styles in Alaveteli core at all and have existing theme styles "Just Work"?

EDIT: Updated screenshots (without mysociety/whatdotheyknow-theme#871)

[garethrees]

I think it would be nicer to render this within alaveteli_pro/info_requests/after_actions?

diff --git a/app/views/alaveteli_pro/info_requests/_after_actions.html.erb b/app/views/alaveteli_pro/info_requests/_after_actions.html.erb
index d9b830634..9230df490 100644
--- a/app/views/alaveteli_pro/info_requests/_after_actions.html.erb
+++ b/app/views/alaveteli_pro/info_requests/_after_actions.html.erb
@@ -1,3 +1,7 @@
+<%= render partial: 'request/reply',
+           locals: { info_request: info_request,
+                     last_response: last_response } %>
+
 <div id="after_actions" class="after-actions">
   <ul class="action-menu after-actions__action-menu">
     <li>
diff --git a/app/views/request/show.html.erb b/app/views/request/show.html.erb
index b184ec6b3..1e7b1fc47 100644
--- a/app/views/request/show.html.erb
+++ b/app/views/request/show.html.erb
@@ -54,9 +54,6 @@
     <div class="request-footer__action-bar-container">
       <div class="request-footer__action-bar__actions">
         <% if @in_pro_area %>
-          <%= render partial: 'request/reply',
-                     locals: { info_request: @info_request,
-                               last_response: @last_response } %>
           <%= render partial: 'alaveteli_pro/info_requests/after_actions',
                      locals: { info_request: @info_request,
                                last_response: @last_response } %>

[garethrees]

Keep this newline

[wrightmartin]

Couple of super minor comments inline, but I think the main thing that would be great to address is making this zero-config for re-users. I can see we've made an update to WDTK (mysociety/whatdotheyknow-theme#871), but we're going to have to remember to do this for all themes.

Is there a better way? If the answer is "Not without a load of effort", then I guess we'll have to settle for this. Have you had a look at whether we can include the styles in Alaveteli core at all and have existing theme styles "Just Work"?

Yeah this is a tough one - for reasons we didn't include button styles in the core but this means, as you say, additions like this aren't zero-config, without knowing more about how the various cobrands have implemented their own button styles any solution is going to be a guess if it will work, eg if we add a button style we think is a sensible default we still don't know how it will interact with their own styles they've implemented. We could add styles to alaveletitheme with the idea they could cherry-pick them but that still results in them having to do something

Also if we add a button style for this one, it's then an exception as no other buttons have styles so it ends up a bit weird.

In a perfect world we'd revisit the theming as I think the white-label ended up a bit too spartan in retrospect. Sorry for the long reply, I'll try and get to the smaller feedback stuff before the end of the sprint

[garethrees]

without knowing more about how the various cobrands have implemented their own button styles any solution is going to be a guess if it will work … Also if we add a button style for this one, it's then an exception as no other buttons have styles so it ends up a bit weird.

Gotcha, let's go with current approach.

We could add styles to alaveletitheme with the idea they could cherry-pick them but that still results in them having to do something

Yeah, do this so that it makes life easier for everyone. We'll also want a line in the "Upgrade notes" section of the changelog to remind reusers to add this.

Sorry for the long reply

No problems here – I'd rather us document this stuff for future reference.

I'll try and get to the smaller feedback stuff before the end of the sprint

Cheers!

[wrightmartin]

Pushed those changes now 👍

[garethrees]

Closed to reduce overwhelm of high WIP. Can reopen in the future if necessary.