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 commit list/view on mobile #19712

Merged
merged 10 commits into from
May 16, 2022
Merged

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented May 15, 2022

  • This is a continuation on the work I've done for improving mobile experience on Gitea.
  • The current behavior of going trough the commits list is horrible, each individual item gets it's own row and thereby isn't quite compact as it should be on mobile. The commit view's header is in a bit better state, it's quite only that content is overlapping each other.
  • This patch fixes those problems. Each row in the commit list table will actually take a row in the UI. The commit view's header has now a better organized way of placing the information.

Before

After

@6543 6543 added topic/ui Change the appearance of the Gitea UI topic/mobile labels May 15, 2022
@6543 6543 added this to the 1.17.0 milestone May 15, 2022
@Gusted Gusted added the type/enhancement An improvement of existing functionality label May 15, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #19712 (06beba8) into main (59b30f0) will decrease coverage by 0.06%.
The diff coverage is 43.13%.

@@            Coverage Diff             @@
##             main   #19712      +/-   ##
==========================================
- Coverage   47.39%   47.33%   -0.07%     
==========================================
  Files         956      958       +2     
  Lines      133115   133595     +480     
==========================================
+ Hits        63093    63235     +142     
- Misses      62393    62695     +302     
- Partials     7629     7665      +36     
Impacted Files Coverage Δ
cmd/hook.go 7.11% <0.00%> (ø)
cmd/serv.go 2.33% <0.00%> (ø)
cmd/web_acme.go 0.00% <0.00%> (ø)
models/error.go 35.87% <ø> (-0.90%) ⬇️
models/org.go 62.99% <0.00%> (ø)
models/statistic.go 0.00% <0.00%> (ø)
modules/context/permission.go 26.08% <0.00%> (ø)
modules/convert/issue_comment.go 41.86% <0.00%> (ø)
modules/doctor/dbconsistency.go 4.54% <0.00%> (-0.52%) ⬇️
modules/git/repo_compare.go 65.19% <0.00%> (-1.78%) ⬇️
... and 134 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 761d4f4...06beba8. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 15, 2022
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.

Seems fine.
The only thing I noticed is:
Do we perhaps still want to display the (un)verified icon when present?
That is so small that it can fit even into such a small screen and lets users know whether to trust this commit.

@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 May 15, 2022
@Gusted
Copy link
Contributor Author

Gusted commented May 15, 2022

Hmm not sure where to place such item, but definitely a good addition to still have the icon.

@delvh
Copy link
Member

delvh commented May 15, 2022

I can see two options for that:
a) Directly after the author and before the commit title
b) Directly after the commit title and before the commit date

Which one do you prefer? Or did I miss an option?

@Gusted
Copy link
Contributor Author

Gusted commented May 15, 2022

I think b makes the most sense logically.

@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 May 16, 2022
@6543 6543 merged commit bcf13b6 into go-gitea:main May 16, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 17, 2022
* giteaofficial/main:
  ContainerRegistry - removed Basic Auth header (go-gitea#19735)
  [skip ci] Updated translations via Crowdin
  Add changelog for v1.16.8 (go-gitea#19724) (go-gitea#19730)
  Improve commit list/view on mobile (go-gitea#19712)
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

I appreciate the work done in this PR and on improving the mobile UX overall.
One question, though: won't it be confusing to users (such as myself) used to seeing commit hashes along with commit messages when suddenly (ref img After-1) there's just commit messages left and it's necessary to click to see the actual hash as well as commit signing status, that is visually indicated by the colour of the frame around the commit message that's now hidden..?
I wish I could bring this up before this was merged but hey, no biggie, just a thought.

@Gusted
Copy link
Contributor Author

Gusted commented May 21, 2022

One question, though: won't it be confusing to users (such as myself) used to seeing commit hashes along with commit messages when suddenly (ref img After-1) there's just commit messages left and it's necessary to click to see the actual hash as well as commit signing status, that is visually indicated by the colour of the frame around the commit message that's now hidden..?

The commit hash is simply too long to be shown on a screen where horizontal space is very limited. In my viewpoint, I never really use the commit hash from such list unless I want to have a commit hash from a specific commit(which is now on mobile one more click away). Yeah I wanted to some work on commit signing status but got stalled by school, but now it's already merged 🤷🏽.

@Gusted Gusted deleted the fix-ui-commit-mobile branch May 21, 2022 13:52
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

The commit hash is simply too long to be shown on a screen where horizontal space is very limited. In my viewpoint, I never really use the commit hash from such list unless I want to have a commit hash from a specific commit(which is now on mobile one more click away).

understood, that's fair, I guess it can get used to. mobile is not a workstation anyway (convergence yay)..

Yeah I wanted to some work on commit signing status but got stalled by school, but now it's already merged 🤷🏽.

oh..it's fine then, really. perhaps another time?
thanks for the improvements anyway.

AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
- This is a continuation on [the work](go-gitea#19546) I've done for improving mobile experience on Gitea.
- The current behavior of going trough the commits list is horrible, each individual item gets it's own row and thereby isn't quite compact as it should be on mobile. The commit view's header is in a bit better state, it's quite only that content is overlapping each other.
- This patch fixes those problems. Each row in the commit list table will actually take a row in the UI. The commit view's header has now a better organized way of placing the information.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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/mobile topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants