From 208647b9279469e576570e441ae65d838ea02fbf Mon Sep 17 00:00:00 2001 From: Pradeep CE (cep) Date: Feb 23 2017 21:51:34 +0000 Subject: [PATCH 1/4] Show icon to jump to changed file from diff When viewing a large diff in a PR, it can be helpful to see the context of the changes. Display a small icon beside the diff's changed lines, which when clicked will open the changed file (at the relevant line number) in a new tab. --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index 255c040..43b397a 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -67,6 +67,15 @@ html padding-right:3px; } +.open_changed_file_icon_wrap { + /* Hidden until user hovers over it */ + visibility: hidden; +} + +.open_changed_file_icon { + width: 15px; +} + .alert-info pre { background-color:#CFE5F0; diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 1813175..fb66894 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -1393,6 +1393,16 @@ $("[data-line-number]").click(function (ev) { $(document).on('click', '#pr-tabs a', function() { window.location.hash = $(this).attr('href'); }); + +// Show an icon to open the changed file, when the user hovers over the +// @@ -x,y +x,y @@ line in the diff. Clicking this icon opens the file (at the +// relevant line number) in a new tab. +$(document).on("mouseenter", "td.cell2", function(){ + $(this).find("a.open_changed_file_icon_wrap").css('visibility', 'visible'); +}); +$(document).on("mouseleave", "td.cell2", function() { + $(this).find("a.open_changed_file_icon_wrap").css('visibility', 'hidden'); +}); diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index 5ad6c74..ab4bb1d 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -32,6 +32,7 @@ import pagure.lib import pagure.forms from pagure import (APP, SESSION, authenticated, is_repo_committer) +from flask import url_for # Jinja filters @@ -140,6 +141,26 @@ def format_loc(loc, commit=None, filename=None, tree_id=None, prequest=None, continue if line.startswith('')[1] + if prequest: + rangeline = line.split( + '@@ ')[1] \ + or None + if rangeline: + rangeline = rangeline.split(' @@')[0] + linenumber = rangeline.split('+')[1].split(',')[0] + line = line + ' ' + \ + '' output.append('
%s
' % line) output.append('') From c90835f7842854fa7b63497571c88f4de6e539ed Mon Sep 17 00:00:00 2001 From: Pradeep CE (cep) Date: Feb 23 2017 21:51:34 +0000 Subject: [PATCH 2/4] Visual and logical improvements for 'jump to file from diff' feature - Prevent scrollbars from appearing on the code line for some zoom levels. - Make the 'open changed file' icon the same size as the other icons and improve it's layout. - Improve logic for extracting the 'lines changed' line in the diff. --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index 43b397a..dc6d0a3 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -43,6 +43,9 @@ html .code_table tr td pre{ padding: 0; margin: 0; + /* prevent scrollbars appearing on each line when + zooming in some cases */ + overflow: hidden; } .code_table tr .cell1{ @@ -73,7 +76,9 @@ html } .open_changed_file_icon { - width: 15px; + /* some adjustments to make the icon look better */ + font-size: 13px !important; /* default size turns out too small here */ + top: 3px !important; /* work around a small vertical misalignment */ } .alert-info pre diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index ab4bb1d..30fa47c 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -32,7 +32,6 @@ import pagure.lib import pagure.forms from pagure import (APP, SESSION, authenticated, is_repo_committer) -from flask import url_for # Jinja filters @@ -142,14 +141,14 @@ def format_loc(loc, commit=None, filename=None, tree_id=None, prequest=None, if line.startswith('')[1] if prequest: - rangeline = line.split( - '@@ ')[1] \ - or None + rangeline = line.partition('font-weight: bold">@@ ')[2] \ + if line.partition('font-weight: bold">@@ ')[1] == \ + 'font-weight: bold">@@ ' else None if rangeline: rangeline = rangeline.split(' @@')[0] linenumber = rangeline.split('+')[1].split(',')[0] line = line + '  Date: Feb 23 2017 21:51:34 +0000 Subject: [PATCH 3/4] Add tests for 'open changed file' icon in PR diffs --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 01ac157..63861b1 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -254,6 +254,13 @@ class PagureFlaskForktests(tests.Modeltests): self.assertIn( 'title="View file as of 2a552b">sources', output.data) + # Test if the `open changed file icon` is displayed. + self.assertIn( + 'class="open_changed_file_icon_wrap">' + '', output.data) + @patch('pagure.lib.notify.send_email') def test_merge_request_pull_FF(self, send_email): """ Test the merge_request_pull endpoint with a FF PR. """ @@ -1422,6 +1429,12 @@ index 0000000..2a552bb output.data) self.assertIn('

Test Initial Comment

', output.data) + # Test if the `open changed file icon` is displayed. + self.assertIn( + 'class="open_changed_file_icon_wrap">' + '', output.data) # Case 2 - Add an empty initial comment data = { From 5d5118426adee2f6fef525107036c2a8f5fbaa48 Mon Sep 17 00:00:00 2001 From: Pradeep CE (cep) Date: Feb 23 2017 21:51:34 +0000 Subject: [PATCH 4/4] Show hand pointer when hovering over 'open changed file' icon --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index dc6d0a3..f042b34 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -81,6 +81,10 @@ html top: 3px !important; /* work around a small vertical misalignment */ } +.open_changed_file_icon:hover { + cursor: pointer; +} + .alert-info pre { background-color:#CFE5F0;