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.

Sign up for GitHub

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

Jump to bottom

Filter Repositories by type #29231

Merged
merged 25 commits into from
Mar 3, 2024
Merged

Filter Repositories by type #29231

merged 25 commits into from
Mar 3, 2024

Conversation

zokkis
Copy link
Contributor

@zokkis zokkis commented Feb 18, 2024 •
edited
Loading

Filter Repositories by type (resolves #1170, #1318)

before:
image

after:
image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 18, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 18, 2024
@zokkis zokkis force-pushed the feature/filter-repo-by-type branch from d55b6de to 04c51f9 Compare February 18, 2024 00:32
@silverwind silverwind added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 18, 2024
@silverwind
Copy link
Member

silverwind commented Feb 19, 2024 •
edited
Loading

Seems to work well. Maybe we can add one more option pair for Template / Not Template. It's in the repo settings:

image

@silverwind
Copy link
Member

Translation strings are missing, otherwise LGTM.

image

@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 21, 2024
@silverwind
Copy link
Member

silverwind commented Feb 21, 2024 •
edited
Loading

BTW the UI could be improved to use tri-state checkboxes (true, false, indeterminate), it would save half the vertical space and half the translations. Not a requirement but would be nice to have. Unfortunately, indeterminate checkbox does require a bit of JS and can not be done from HTML only.

@zokkis zokkis closed this Feb 21, 2024
@zokkis zokkis reopened this Feb 21, 2024
@zokkis
Copy link
Contributor Author

zokkis commented Feb 21, 2024

BTW the UI could be improved to use tri-state checkboxes (true, false, indeterminate), it would save half the vertical space and half the translations. Not a requirement but would be nice to have. Unfortunately, indeterminate checkbox does require a bit of JS and can not be done from HTML only.

The js should not be a problem, but every click in the menu the menu is closed and a the page gets reloaded and I dont know a solution to this

@silverwind
Copy link
Member

Let's keep current solution then. Indetermine checkboxes are uncommon in UIs, so they might be confusing UX.

wxiaoguang
wxiaoguang previously requested changes Feb 22, 2024
View reviewed changes
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.

At first glance, many things need to be correctly escaped.

@zokkis zokkis force-pushed the feature/filter-repo-by-type branch from f127130 to 6662065 Compare February 27, 2024 23:43
@zokkis zokkis requested a review from wxiaoguang February 27, 2024 23:44
@6543
Copy link
Member

6543 commented Feb 28, 2024

INFO: if #29461 got merged, I'll push a commit to fix this pull :)

@6543
Copy link
Member

6543 commented Feb 28, 2024 

{{if and .IsPrivate.Has (not .IsPrivate.Value)}}checked{{end}}

or

{{if (not (.IsPrivate.ValueOrDefault true))}}checked{{end}}

the later one is more efficient ... but i dont know what should be prefered ...

@silverwind
Copy link
Member

Latter seems more clear to me.

@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 28, 2024
@silverwind
Copy link
Member

Some lint errors. Related to @6543's recent boolean changes?

@6543
Copy link
Member

6543 commented Mar 3, 2024

uh 👀

@6543
Copy link
Member

6543 commented Mar 3, 2024

yes latest refactores caused it ... addressing it

@6543
Copy link
Member

6543 commented Mar 3, 2024

@wxiaoguang are you gona re-review it?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 3, 2024 •
edited
Loading

Inputting a keyword and clicking the search button will clean all types filters, is that expected?

I haven't got time to test. If this concern has been addressed, then it looks good to me. Feel free to merge.

@6543 6543 enabled auto-merge (squash) March 3, 2024 09:57
@6543 6543 added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 3, 2024
@6543 6543 merged commit e3524c6 into go-gitea:main Mar 3, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 3, 2024
@zokkis zokkis deleted the feature/filter-repo-by-type branch March 3, 2024 11:05
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 4, 2024
@zjjhot
Merge remote-tracking branch 'upstream/main'
b9e4a44 
* upstream/main:
  Replace some `gt-` classes with `tw-` (go-gitea#29570)
  Enable/disable owner and repo projects independently (go-gitea#28805)
  Add an trailing slash to dashboard links (go-gitea#29555)
  Extend issue template yaml engine (go-gitea#29274)
  [skip ci] Updated licenses and gitignores
  Fix workflow trigger event IssueChangeXXX bug (go-gitea#29559)
  Fix 500 when pushing release to an empty repo (go-gitea#29554)
  Update js and py dependencies, bump python (go-gitea#29561)
  Filter Repositories by type (go-gitea#29231)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 approved these changes

@silverwind silverwind silverwind approved these changes

@wxiaoguang wxiaoguang Awaiting requested review from wxiaoguang

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

Filter repositories by type
6 participants
@zokkis @silverwind @lunny @wxiaoguang @6543 @GiteaBot