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

Truncate long commit message header #10301

Merged
merged 8 commits into from
Feb 17, 2020

Conversation

zeripath
Copy link
Contributor

  • Make the message-wrapper style no longer require being inside a commit-list.
  • Wrap the header on the commit page in a message-wrapper

Fix #8900
Fix #10209

@zeripath
Copy link
Contributor Author

zeripath commented Feb 16, 2020

damn I've just seen another place that this doesn't fix.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 16, 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 Feb 16, 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 Feb 16, 2020
@zeripath zeripath changed the title Truncate long commit message header WIP: Truncate long commit message header Feb 16, 2020
@zeripath zeripath added the pr/wip This PR is not ready for review label Feb 16, 2020
@zeripath
Copy link
Contributor Author

zeripath commented Feb 16, 2020

Hmm... If I stick this style on the appropriate element in templates/repo/view-list.tmpl then we get appropriate sizes.

style="width: min(calc((max(100vw, 768px) - 768px) * 800 + 20rem), calc((max(100vw, 992px) - 992px) * 800 + 360px), calc((max(100vw, 1200px) - 1200px) * 800 + 500px), calc((min(100vw, 1200px) - 1200px) * (0 - 800) + 700px));" 

@zeripath
Copy link
Contributor Author

Current problems:
stilloverflowingbadly
toofaraway
tooclose

@zeripath
Copy link
Contributor Author

Screenshot from 2020-02-17 11-02-48

OK I think it's fixed.

@silverwind could you take a look? I've used 90vw - I dunno if that's allowed?

@zeripath zeripath changed the title WIP: Truncate long commit message header Truncate long commit message header Feb 17, 2020
@zeripath zeripath removed the pr/wip This PR is not ready for review label Feb 17, 2020
@silverwind
Copy link
Member

Does it work with the CI icon shown? I'm still not a fan of CSS truncation here in general, it's always rather hackish because it's a table, not a flexbox. We could opt to do what GitHub does and make the header line a flexbox but that'd require a quite a few changes.

@silverwind
Copy link
Member

90vw seems like an arbitrary value but if works in most cases, I guess I'm fine with such hacks until we can convert it to flexbox (then it's just flex: 1 on the parent and a wrapping <div> with overflow: hidden + ellipsis).

@zeripath
Copy link
Contributor Author

zeripath commented Feb 17, 2020

@silverwind I guesss calc(100vw - 70px) might work better?


I've done that now.

@codecov-io
Copy link

Codecov Report

Merging #10301 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10301      +/-   ##
==========================================
- Coverage   43.74%   43.71%   -0.03%     
==========================================
  Files         585      585              
  Lines       81015    81015              
==========================================
- Hits        35441    35419      -22     
- Misses      41179    41212      +33     
+ Partials     4395     4384      -11
Impacted Files Coverage Δ
modules/markup/markup.go 76.81% <ø> (ø) ⬆️
modules/markup/markdown/goldmark.go 66.07% <ø> (ø) ⬆️
services/pull/check.go 35.97% <0%> (-17.08%) ⬇️
services/pull/temp_repo.go 29.05% <0%> (-2.57%) ⬇️
services/pull/patch.go 60.37% <0%> (-2.52%) ⬇️
models/notification.go 63.29% <0%> (-1.9%) ⬇️
models/pull.go 41.82% <0%> (-0.56%) ⬇️
modules/log/event.go 65.64% <0%> (+1.02%) ⬆️
... and 6 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 2d4edcd...fe4baa0. Read the comment docs.

@lafriks lafriks merged commit 314740e into go-gitea:master Feb 17, 2020
@zeripath zeripath deleted the fix-8900-wrap-long-message branch February 17, 2020 17:48
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 17, 2020
* Truncate long commit message header

* Fix overflow in view commit table

* Use @media less

* Further improvements

* Fix the commit message on small screens

* adjust width of minimal table
sapk pushed a commit that referenced this pull request Feb 18, 2020
* Truncate long commit message header

* Fix overflow in view commit table

* Use @media less

* Further improvements

* Fix the commit message on small screens

* adjust width of minimal table
@zeripath zeripath added the backport/done All backports for this PR have been created label Mar 6, 2020
@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
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
7 participants