Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not show arrows on comment diffs on pull comment pages #12434

Merged

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Aug 5, 2020

Prior to this PR it was possible that an expansion arrow could be displayed
on comment diffs displayed on the comments pages of pulls

These arrows would not successfully work because they were not attached to
a commit id - nor can they necessarily be.

This PR prevents these from being shown.

Fix #10851

Signed-off-by: Andrew Thornton [email protected]

Prior to this PR it was possible that an expansion arrow could be displayed
on comment diffs displayed on the comments pages of pulls

These arrows would not successfully work because they were not attached to
a commit id - nor can they necessarily be.

This PR prevents these from being shown.

Fix go-gitea#10851

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added type/bug topic/ui Change the appearance of the Gitea UI labels Aug 5, 2020
@zeripath zeripath added this to the 1.13.0 milestone Aug 5, 2020
@silverwind
Copy link
Member

silverwind commented Aug 5, 2020

Looking at how GH does it, they hide the complete line including the diff markers:

image

image

While I don't get the reason why line expansion could not work in this context, I think it may better to follow GH and remove the whole line as all data in it is already visible (line numbers and file name):

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 5, 2020
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 5, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 7, 2020
@lunny
Copy link
Member

lunny commented Aug 8, 2020

make L-G-T-M work

@lunny lunny merged commit 96add8c into go-gitea:master Aug 8, 2020
@zeripath zeripath deleted the fix-10851-hide-up-arrows-comment-diffs branch August 8, 2020 07:24
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

404 Not found error on blob_excerpt expansion
5 participants