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

Improve PR diff view on mobile #27883

Merged

Conversation

sebastian-sauer
Copy link
Contributor

@sebastian-sauer sebastian-sauer commented Nov 2, 2023

  1. Show diff stats only on large screens

these are already shown in tabs, so no need for this duplicate information on small screens

image
image

  1. Hide viewed files information on small screens

Github does the same and this gives us more free space on small screens

image
image

  1. Review bar now doesn't wrap so we don't need the 77px even on very small screens

(the sticky headers are still working)

image

they are already displayed in tab menu
github hides them on small screens, too
there is currently no wrapping even on very small screens
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 2, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 2, 2023
@sebastian-sauer sebastian-sauer changed the title Feature/improve pr view on mobile Improve PR diff view on mobile Nov 2, 2023
@silverwind silverwind added the type/enhancement An improvement of existing functionality label Nov 2, 2023
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

This PR will remove necessary information:
grafik
The label is part of the sticky header.
So, it will be a regression on non-mobile systems.

@sebastian-sauer
Copy link
Contributor Author

these are already shown in tabs, so no need for this duplicate information (and it takes just screen space)

Good catch. I'll readd the information on non mobile devices.
Do you think it's also a needed part on mobile devices?

@delvh
Copy link
Member

delvh commented Nov 2, 2023

No, there I absolutely agree that we need to slim down the interface as much as possible.

@sebastian-sauer
Copy link
Contributor Author

Updated codes and screenshots - large screens will still show the information.

@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 Nov 2, 2023
web_src/css/review.css Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 3, 2023
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Haven't tested (not sure whether it still needs "height match"). Overall it looks good to me, so vote my approval.

@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 Nov 16, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 16, 2023
@lunny lunny merged commit 49dddd8 into go-gitea:main Nov 16, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Nov 16, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 16, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 16, 2023
* giteaofficial/main:
  Improve PR diff view on mobile (go-gitea#27883)
  Improve graceful manager code/comment (go-gitea#28063)
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
1. Show diff stats only on large screens

these are already shown in tabs, so no need for this duplicate
information on small screens


![image](https://github.com/go-gitea/gitea/assets/1135157/1287839d-7490-42eb-a17e-d526dc0bfd9e)

![image](https://github.com/go-gitea/gitea/assets/1135157/e9dcd89d-ed4d-4945-a7aa-4e6fc6d9c3a2)


2. Hide viewed files information on small screens

Github does the same and this gives us more free space on small screens


![image](https://github.com/go-gitea/gitea/assets/1135157/e90b042f-fffb-4f79-a5ae-cd480c9d8334)

![image](https://github.com/go-gitea/gitea/assets/1135157/d2480ffe-58f2-4694-8ae1-a2ab0aae14d4)


3. Review bar now doesn't wrap so we don't need the 77px even on very
small screens

(the sticky headers are still working)


![image](https://github.com/go-gitea/gitea/assets/1135157/42b19b2b-73ef-4b88-8680-c555879b363b)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 14, 2024
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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants