[v3,8/9] templates: Use buttons for patch download links

Submitted by Stephen Finucane on March 7, 2017, 7:46 p.m.

Details

Message ID 20170307194624.21582-9-stephen@that.guru
State Superseded
Headers show

Commit Message

Stephen Finucane March 7, 2017, 7:46 p.m.
We'll make use of this to enable downloading of patches with mboxes.

In addition, the 'hide' link for patches and cover letters is removed.
There's no way to enable this by default and the use cases are dubious
at best.

Signed-off-by: Stephen Finucane <stephen@that.guru>
---
 patchwork/templates/patchwork/submission.html | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Daniel Axtens March 9, 2017, 6:38 a.m.
Stephen Finucane <stephen@that.guru> writes:

So, I've polled users, and the feedback is that people don't
particularly care about the button vs link thing or the placement on the
right, (so long as they can still right click to get a link), but people
would really like buttons up the top of the page as well as at the top
of the patch.

I'm happy to do that as a follow up if you'd like.
I've also got a few other UI requests which I'll do in a series.

In my testing, it looks like the buttons act as toggle buttons rather
than momentary (i.e. they 'stay pressed') - does this happen to you too?
I use Chrome.

Regards,
Daniel

> We'll make use of this to enable downloading of patches with mboxes.
>
> In addition, the 'hide' link for patches and cover letters is removed.
> There's no way to enable this by default and the use cases are dubious
> at best.
>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> ---
>  patchwork/templates/patchwork/submission.html | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index 21c345f..66c1159 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -281,14 +281,16 @@ function toggle_div(link_id, headers_id)
>  {% if submission.diff %}
>  <h2>
>   Patch
> - <a href="javascript:toggle_div('hide-patch', 'patch')" id="hide-patch">hide</a></span>
> - <span>|</span>
> - <a href="{% url 'patch-raw' patch_id=submission.id %}"
> -   >download patch</a>
> - <span>|</span>
> - <a href="{% url 'patch-mbox' patch_id=submission.id %}"
> -   >download mbox</a>
> + <div class="btn-group pull-right">
> +  <a href="{% url 'patch-raw' patch_id=submission.id %}"
> +   class="btn btn-default" role="button" data-toggle="tooltip"
> +   title="Download patch diff">diff</a>
> +  <a href="{% url 'patch-mbox' patch_id=submission.id %}"
> +   class="btn btn-default" role="button" data-toggle="tooltip"
> +   title="Download patch mbox">mbox</a>
> + </div>
>  </h2>
> +
>  <div id="patch" class="patch">
>  <pre class="content">
>  {{ submission|patchsyntax }}
> -- 
> 2.9.3
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane March 10, 2017, 5:40 p.m.
On Thu, 2017-03-09 at 17:38 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen@that.guru> writes:
> 
> So, I've polled users, and the feedback is that people don't
> particularly care about the button vs link thing or the placement on
> the
> right, (so long as they can still right click to get a link), but
> people
> would really like buttons up the top of the page as well as at the
> top
> of the patch.
> 
> I'm happy to do that as a follow up if you'd like.
> I've also got a few other UI requests which I'll do in a series.

That sounds like the best option, yes. I'd be happy to review that
series.

> In my testing, it looks like the buttons act as toggle buttons rather
> than momentary (i.e. they 'stay pressed') - does this happen to you
> too?
> I use Chrome.

They're not pressed - they have focus. You'd see the same thing if you
tabbed down to them. Unfortunately there's no way around this without
some hacky JavaScript which isn't worth it (IMO, at least :))

Stephen

Patch hide | download patch | download mbox

diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index 21c345f..66c1159 100644
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -281,14 +281,16 @@  function toggle_div(link_id, headers_id)
 {% if submission.diff %}
 <h2>
  Patch
- <a href="javascript:toggle_div('hide-patch', 'patch')" id="hide-patch">hide</a></span>
- <span>|</span>
- <a href="{% url 'patch-raw' patch_id=submission.id %}"
-   >download patch</a>
- <span>|</span>
- <a href="{% url 'patch-mbox' patch_id=submission.id %}"
-   >download mbox</a>
+ <div class="btn-group pull-right">
+  <a href="{% url 'patch-raw' patch_id=submission.id %}"
+   class="btn btn-default" role="button" data-toggle="tooltip"
+   title="Download patch diff">diff</a>
+  <a href="{% url 'patch-mbox' patch_id=submission.id %}"
+   class="btn btn-default" role="button" data-toggle="tooltip"
+   title="Download patch mbox">mbox</a>
+ </div>
 </h2>
+
 <div id="patch" class="patch">
 <pre class="content">
 {{ submission|patchsyntax }}