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

Add 'Alt + click' feature to exclude labels #8199

Merged
merged 19 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1394,8 +1394,12 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {

if opts.LabelIDs != nil {
for i, labelID := range opts.LabelIDs {
sess.Join("INNER", fmt.Sprintf("issue_label il%d", i),
fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID))
if labelID > 0 {
sess.Join("INNER", fmt.Sprintf("issue_label il%d", i),
fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID))
} else {
sess.Where("issue.id not in (select issue_id from issue_label where label_id = ?)", -labelID)
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion models/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type Label struct {
IsChecked bool `xorm:"-"`
QueryString string
IsSelected bool
IsExcluded bool
Copy link
Member

Choose a reason for hiding this comment

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

Why we need this new column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

So this will be shared by all the users of this repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it same with IsSelected

I dont know if its correct

Copy link
Member

Choose a reason for hiding this comment

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

@jaqra This kind of value is not necessarily tied to the model. Perhaps it is better placed in the request context. I didn't check your code, but if the column is transient, you should at least use xorm:="-" to avoid adding it to the database.

Copy link
Contributor

Choose a reason for hiding this comment

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

On this point, and not necessarily for your PR, why is IsSelected not xorm:"-" too. I can't see why we would want that persisted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zeripath those two lines (QueryString and IsSelected) added #5786 as rework of #3438. Parhaps no one did not see that ?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like an error to me. As @lunny 've said, those fields are related to one user's session and should not be affected by other user's work (perhaps nobody have noticed because the value from the database is not actually used?)

Copy link
Member

Choose a reason for hiding this comment

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

We should send another PR to add xorm ignore tag on the fields #5786 added wrong.

Copy link
Member

@guillep2k guillep2k Oct 22, 2019

Choose a reason for hiding this comment

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

We should send another PR to add xorm ignore tag on the fields #5786 added wrong.

@lunny done in #8633

}

// APIFormat converts a Label to the api.Label format
Expand All @@ -96,7 +97,10 @@ func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64)
for _, s := range currentSelectedLabels {
if s == label.ID {
labelSelected = true
} else if s > 0 {
} else if -s == label.ID {
labelSelected = true
label.IsExcluded = true
} else if s != 0 {
labelQuerySlice = append(labelQuerySlice, strconv.FormatInt(s, 10))
}
}
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ issues.delete_branch_at = `deleted branch <b>%s</b> %s`
issues.open_tab = %d Open
issues.close_tab = %d Closed
issues.filter_label = Label
issues.filter_label_exclude = `Use <code>alt</code> + <code>click/enter</code> to exclude labels`
issues.filter_label_no_select = All labels
issues.filter_milestone = Milestone
issues.filter_milestone_no_select = All milestones
Expand Down
2 changes: 2 additions & 0 deletions public/css/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,8 @@ footer .ui.left,footer .ui.right{line-height:40px}
.repository .filter.menu .label.color{border-radius:3px;margin-left:15px;padding:0 8px}
.repository .filter.menu .octicon{float:left;margin:5px -7px 0 -5px;width:16px}
.repository .filter.menu.labels .octicon{margin:-2px -7px 0 -5px}
.repository .filter.menu.labels .label-filter .menu .info{display:inline-block;padding:9px 7px 7px 7px;text-align:center;border-bottom:1px solid #ccc;font-size:12px}
.repository .filter.menu.labels .label-filter .menu .info code{border:1px solid #ccc;border-radius:3px;padding:3px 2px 1px 2px;font-size:11px}
.repository .filter.menu .text{margin-left:.9em}
.repository .filter.menu .menu{max-height:300px;overflow-x:auto;right:0!important;left:auto!important}
.repository .filter.menu .dropdown.item{margin:1px;padding-right:0}
Expand Down
35 changes: 33 additions & 2 deletions public/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3096,8 +3096,39 @@ function initIssueList() {
},

fullTextSearch: true
})
;
});

$(".menu a.label-filter-item").each(function() {
$(this).click(function(e) {
if (e.altKey) {
const href = $(this).attr("href");
const id = $(this).data("label-id");

const regStr = "labels=(-?[0-9]+%2c)*(" + id + ")(%2c-?[0-9]+)*&";
const newStr = "labels=$1-$2$3&";

window.location = href.replace(new RegExp(regStr), newStr);
}
});
});

$(".menu .ui.dropdown.label-filter").keydown(function(e) {
if (e.altKey && e.keyCode == 13) {
const selectedItems = $(".menu .ui.dropdown.label-filter .menu .item.selected");

if (selectedItems.length > 0) {
const item = $(selectedItems[0]);

const href = item.attr("href");
const id = item.data("label-id");

const regStr = "labels=(-?[0-9]+%2c)*(" + id + ")(%2c-?[0-9]+)*&";
const newStr = "labels=$1-$2$3&";

window.location = href.replace(new RegExp(regStr), newStr);
}
}
});
}
function cancelCodeComment(btn) {
const form = $(btn).closest("form");
Expand Down
17 changes: 17 additions & 0 deletions public/less/_repository.less
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,23 @@
margin: -2px -7px 0 -5px;
}

&.labels {
.label-filter .menu .info {
display: inline-block;
padding: 9px 7px 7px 7px;
text-align: center;
border-bottom: 1px solid #cccccc;
font-size: 12px;

code {
border: 1px solid #cccccc;
border-radius: 3px;
padding: 3px 2px 1px 2px;
font-size: 11px;
}
}
}

.text {
margin-left: 0.9em;
}
Expand Down
5 changes: 3 additions & 2 deletions templates/repo/issue/list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,16 @@
<div class="ten wide right aligned column">
<div class="ui secondary filter stackable menu labels">
<!-- Label -->
<div class="ui {{if not .Labels}}disabled{{end}} dropdown jump item" style="margin-left: auto">
<div class="ui {{if not .Labels}}disabled{{end}} dropdown jump item label-filter" style="margin-left: auto">
<span class="text">
{{.i18n.Tr "repo.issues.filter_label"}}
<i class="dropdown icon"></i>
</span>
<div class="menu">
<span class="info">{{.i18n.Tr "repo.issues.filter_label_exclude" | Safe}}</span>
<a class="item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_label_no_select"}}</a>
{{range .Labels}}
<a class="item has-emoji" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}"><span class="octicon {{if .IsSelected}}octicon-check{{end}}"></span><span class="label color" style="background-color: {{.Color}}"></span> {{.Name}}</a>
<a class="item has-emoji label-filter-item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}" data-label-id="{{.ID}}"><span class="octicon {{if .IsExcluded}}octicon-circle-slash{{else if .IsSelected}}octicon-check{{end}}"></span><span class="label color" style="background-color: {{.Color}}"></span> {{.Name}}</a>
{{end}}
</div>
</div>
Expand Down