-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Implement the feature that close issuse as archived/merged/resolved #23522
Conversation
models/issues/issue.go
Outdated
// IssueStateOpen | ||
IssueStateOpen IssueCloseState = iota | ||
// IssueStateLocked | ||
IssueStateLocked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure whether this is the final design (I see this PR is WIP), but I'd like to have quick comment about this design: I guess the "Locked" should be a separate state. For example: Merged, Merged+Locked
{{.locale.Tr "repo.issues.close_issue"}} | ||
</button> | ||
{{end}} | ||
|
||
<div id="state-dropdown" class="ui dropdown jump item tooltip gt-px-3" tabindex="-1" data-content=""> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you need this https://fomantic-ui.com/modules/dropdown.html#floating , instead of re-inventing wheels
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's exactly what i need😂
# Conflicts: # templates/repo/issue/view_content.tmpl # web_src/css/repository.css
web_src/js/features/aria.js
Outdated
// for menu, when the popup is hidden, no need to keep the aria-activedescendant, and clear the active/selected item | ||
} else if (!isComboBox && !needKeepAtive) { | ||
// for menu in most cases, when the popup is hidden, no need to keep the aria-activedescendant, and clear the active/selected item | ||
// if you really need to keep the aria-activedescendant, plz add a className 'keep-active' to the dropdown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
menu
doesn't keep last selected item, and it would break some screen readers IIRC.
You need to make it a combobox
.
For example: class=js-aria-comobox
, then isCombobox=true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, a quick comment~That's exactly what I wanted to ask you.
web_src/js/features/aria.js
Outdated
@@ -38,6 +38,8 @@ function attachOneDropdownAria($dropdown) { | |||
const listPopupRole = isComboBox ? 'listbox' : 'menu'; | |||
const listItemRole = isComboBox ? 'option' : 'menuitem'; | |||
|
|||
const needKeepAtive = $dropdown.hasClass('keep-active'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wxiaoguang hi, in this case, i need to keep check
icon active even if the menu is hidden and reopened.
So, I add a classname 'keep-active' to the dropdown. But I'm not sure that's a good idea. Do you have better one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have commented above 😂
This comment was marked as off-topic.
This comment was marked as off-topic.
|
||
const ( | ||
// IssueClosedStatusOpen | ||
IssueClosedStatusOpen IssueClosedStatus = iota |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks meanless, can check it by Issue.IsClosed
// IssueClosedStatusResolved | ||
IssueClosedStatusResolved | ||
// IssueClosedStatusMerged | ||
IssueClosedStatusMerged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about add duplicate
and stale
status?
// 37 | ||
CommentTypeCloseAsResolved | ||
// 38 | ||
CommentTypeCloseAsMerged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's better to extend the CommentTypeClose
, instead of adding too many new comment types.
This PR is not good enough, I am trying to rewrite this in a new branch, and will create a new PR replace this when it was done. So close it for now. |
Close #22793
Implement the feature of "Close as archived / resolved / merged"
Create a new issues, open the dropdown, user can select what they want to do.

click the left button, submit with a comment.


in the issue list, we can see what kind of

closed status
in the tooltip