Skip to content

Conversation

@sillyguodong
Copy link
Contributor

@sillyguodong sillyguodong commented Mar 22, 2023

close #23628

Now in ... dropdown, you can expand or collapse all diff files that have loaded.

Screen.Recording.2023-03-26.at.09.02.59.mov

@sillyguodong sillyguodong added type/feature Completely new functionality. Can only be merged if feature freeze is not active. pr/wip This PR is not ready for review labels Mar 22, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 22, 2023
@silverwind
Copy link
Member

Screenshot please.

@delvh
Copy link
Member

delvh commented Mar 22, 2023

Hmm… We should also discuss if this really needs to be backend functionality.
I've understood the request rather as a purely frontend side feature:
As I've understood it, it should simply expand/collapse all files, and not reset the Viewed state.

@gempir
Copy link
Contributor

gempir commented Mar 23, 2023

I agree that not modifying the view state is probably wiser. The Usecase is not that common and not sending a bunch of changed data to the backend reduces load.

So just a open all/fold all button frontendside would probably be enough.

@sillyguodong
Copy link
Contributor Author

thank you guys for your review and suggestion : )
let me try to implement expand/collapse later.

@sillyguodong sillyguodong changed the title View/Unview all changed files Expand/Collapse all changed files Mar 23, 2023
@sillyguodong sillyguodong removed the pr/wip This PR is not ready for review label Mar 24, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 24, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2023

Codecov Report

Merging #23639 (d53cc69) into main (f521e88) will decrease coverage by 0.10%.
The diff coverage is 30.78%.

❗ Current head d53cc69 differs from pull request most recent head acd8c87. Consider uploading reports for the commit acd8c87 to get more accurate results

@@            Coverage Diff             @@
##             main   #23639      +/-   ##
==========================================
- Coverage   47.14%   47.04%   -0.10%     
==========================================
  Files        1149     1163      +14     
  Lines      151446   153738    +2292     
==========================================
+ Hits        71397    72331     +934     
- Misses      71611    72890    +1279     
- Partials     8438     8517      +79     
Impacted Files Coverage Δ
cmd/dump.go 0.66% <0.00%> (-0.01%) ⬇️
cmd/mailer.go 0.00% <0.00%> (ø)
cmd/manager.go 0.00% <0.00%> (ø)
cmd/manager_logging.go 0.00% <0.00%> (ø)
cmd/migrate_storage.go 5.76% <0.00%> (-0.12%) ⬇️
cmd/restore_repo.go 0.00% <0.00%> (ø)
cmd/web.go 0.00% <0.00%> (ø)
models/actions/run.go 1.63% <0.00%> (-0.10%) ⬇️
models/actions/runner.go 1.44% <ø> (ø)
models/organization/org_user.go 80.51% <0.00%> (-12.02%) ⬇️
... and 64 more

... and 108 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@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 Mar 24, 2023
@lunny lunny added topic/ui Change the appearance of the Gitea UI and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 24, 2023
@silverwind
Copy link
Member

Hmm I don't think I like those icon choices. How about moving these actions into the "..." menu? I think they are not all that common to warrant separate always-visible buttons, and inside that menu, no icons are needed 😉.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 25, 2023
@sillyguodong
Copy link
Contributor Author

Hmm I don't think I like those icon choices. How about moving these actions into the "..." menu? I think they are not all that common to warrant separate always-visible buttons, and inside that menu, no icons are needed 😉.

done~

@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 Apr 9, 2023
@silverwind
Copy link
Member

silverwind commented Apr 9, 2023

Tested, works.

I had to slightly abuse the PR to add one semi-related change that adjust border-radius to fit more into our theme for this label:

Before:
Screenshot 2023-04-09 at 14 15 27

After:
Screenshot 2023-04-09 at 14 15 15

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 9, 2023
@lunny lunny merged commit bedad23 into go-gitea:main Apr 9, 2023
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 9, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
@sillyguodong sillyguodong deleted the feature/issue_23628 branch February 29, 2024 03:30
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/feature Completely new functionality. Can only be merged if feature freeze is not active.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unfold/Unview all files

8 participants