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

Hide archived labels by default from the suggestions when assigning labels for an issue #27451

Merged

Conversation

puni9869
Copy link
Member

@puni9869 puni9869 commented Oct 5, 2023

Followup of #27115
Finally closes #25237

Screenshots

Issue Sidebar

image

PR sidebar

image

PR sidebar with archived labels shown

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 5, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 5, 2023
@github-actions github-actions bot added the topic/ui Change the appearance of the Gitea UI label Oct 5, 2023
@delvh delvh added the backport/v1.21 This PR should be backported to Gitea 1.21 label Oct 5, 2023
@delvh delvh changed the title Adding archive label functionality for label selector in issue/pr detail page Hide archived labels from the suggestions when assigning labels for an issue by default Oct 5, 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.

Your approach is one way of doing it.
There is one other possible approach:

I don't see a benefit in assigning archived labels.
They should only be removable from an issue.
It is basically the same approach as in the previous PR:

  • Always show selected labels or unarchived labels (including the archived hint if a label is archived).
  • However, don't show the checkbox to show archived labels, it is simply not present.
  • We may or may not still accept the URL parameter (show-archived=true) and handle it accordingly for power users who want

As we've discussed in person, I'm fine with either approach.

templates/repo/issue/labels/labels_selector_field.tmpl Outdated Show resolved Hide resolved
templates/repo/issue/labels/labels_selector_field.tmpl Outdated Show resolved Hide resolved
@puni9869
Copy link
Member Author

puni9869 commented Oct 6, 2023

Ready for review again.

web_src/js/features/repo-issue-list.js Outdated Show resolved Hide resolved
@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 Oct 7, 2023
@puni9869
Copy link
Member Author

puni9869 commented Oct 8, 2023

can I get one more approval or review

@@ -680,3 +680,10 @@ export function initIssueTemplateCommentEditors($commentForm) {
initCombo($(el));
}
}

export function initArchivedLabelHandler() {
if (!document.querySelector('.archived-label-hint')) return;
Copy link
Member

Choose a reason for hiding this comment

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

Why are we selecting for this hint? Seems brittle. Can the below loop stand on its own without this? If not, at least add a comment explaining the reason for this selector.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will refactor this naming a little bit.

@silverwind
Copy link
Member

silverwind commented Oct 13, 2023

Why is there no [Archived] label besides archived labels? Why not remove this checkbox and just sort archived labels last, always showing them, but with the tag?

I find the current UI confusing, and would rather rewrite the dropdown to a style similar to GitHub.

@delvh
Copy link
Member

delvh commented Oct 13, 2023

Yes, that will be the next step.
As far as I know, @puni9869 is already working on that.
But for now, we need at least something (for 1.21), and can still improve it later on.

Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Not totally happy with it but I guess if you really must land it, go ahead.

@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 Oct 16, 2023
@delvh delvh added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 17, 2023
@delvh delvh changed the title Hide archived labels from the suggestions when assigning labels for an issue by default Hide archived labels by default from the suggestions when assigning labels for an issue Oct 17, 2023
@delvh delvh merged commit 4adc2a8 into go-gitea:main Oct 17, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Oct 17, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Oct 17, 2023
…abels for an issue (go-gitea#27451)

Followup of go-gitea#27115
Finally closes go-gitea#25237

## Screenshots
### Issue Sidebar
<img width="513" alt="image"
src="https://github.com/go-gitea/gitea/assets/80308335/9f7fda2f-5a03-4684-8619-fd3498a95b41">

### PR sidebar
<img width="367" alt="image"
src="https://github.com/go-gitea/gitea/assets/80308335/53db9b64-faec-4a67-91d6-76945596a469">

### PR sidebar with archived labels shown
<img width="352" alt="image"
src="https://github.com/go-gitea/gitea/assets/80308335/9dc5050f-4e69-4f76-bb83-582480a2281e">

---------

Signed-off-by: puni9869 <punitinani1@hotmail.com>
Co-authored-by: silverwind <me@silverwind.io>
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Oct 17, 2023
6543 pushed a commit that referenced this pull request Oct 17, 2023
…abels for an issue (#27451) (#27661)

Backport #27451 by @puni9869

Followup of #27115
Finally closes #25237

## Screenshots
### Issue Sidebar
<img width="513" alt="image"
src="https://github.com/go-gitea/gitea/assets/80308335/9f7fda2f-5a03-4684-8619-fd3498a95b41">

### PR sidebar
<img width="367" alt="image"
src="https://github.com/go-gitea/gitea/assets/80308335/53db9b64-faec-4a67-91d6-76945596a469">

### PR sidebar with archived labels shown
<img width="352" alt="image"
src="https://github.com/go-gitea/gitea/assets/80308335/9dc5050f-4e69-4f76-bb83-582480a2281e">

Signed-off-by: puni9869 <punitinani1@hotmail.com>
Co-authored-by: puni9869 <80308335+puni9869@users.noreply.github.com>
Co-authored-by: silverwind <me@silverwind.io>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 18, 2023
* giteaofficial/main:
  Improve the list header in milestone page (go-gitea#27302)
  Fix poster is not loaded in get default merge message (go-gitea#27657)
  Hide archived labels by default from the suggestions when assigning labels for an issue (go-gitea#27451)
  actions/setup-go use go-version-file (go-gitea#27651)
  Update agit-support.en-us.md (go-gitea#27652)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 15, 2024
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 backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Archive labels
5 participants