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

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

Merged
merged 19 commits into from
Oct 23, 2019
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
34c503c
Add 'Alt + click' feature to exclude labels
metiftikci Sep 16, 2019
d3b650b
:lipstick:
metiftikci Sep 16, 2019
ae01e37
Merge branch 'master' into exclude_label
Sep 16, 2019
0e69c42
Merge branch 'master' into exclude_label
Sep 17, 2019
1324fbc
Add info
metiftikci Oct 16, 2019
6e36ac0
fix label colors
metiftikci Oct 16, 2019
65076ac
fix info css
metiftikci Oct 16, 2019
8a2d9f6
mave funtion and add enter event
metiftikci Oct 16, 2019
d0fc7bc
change class name
metiftikci Oct 16, 2019
afa6944
add message to locale
metiftikci Oct 16, 2019
e714635
Add `xorm:"-"` for Label.IsExcluded struct
Oct 19, 2019
8d253a0
Merge branch 'master' into exclude_label
lunny Oct 22, 2019
e09d335
Merge branch 'master' into exclude_label
Oct 22, 2019
a397248
Merge branch 'master' into exclude_label
lunny Oct 23, 2019
afe39f9
Merge branch 'master' into exclude_label
Oct 23, 2019
f723afe
try to fix format
Oct 23, 2019
82b0b45
Merge branch 'master' into exclude_label
Oct 23, 2019
d86ff3d
Merge branch 'master' into exclude_label
lafriks Oct 23, 2019
d9ca015
Merge branch 'master' into exclude_label
Oct 23, 2019
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
View file
Open in desktop
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
View file
Open in desktop
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.

Its for show blocked icon

https://github.com/jaqra/gitea/blob/34c503c5573995ad0df4803df86647341b0d5de7/templates/repo/issue/list.tmpl#L53

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 •
edited
Loading

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
16 changes: 16 additions & 0 deletions public/js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3122,3 +3122,19 @@ function onOAuthLoginClick() {
oauthNav.show();
},5000);
}

(function addLabelFilterClickEvents() {
lafriks marked this conversation as resolved.
Show resolved Hide resolved
$(".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);
}
});
});
})();
2 changes: 1 addition & 1 deletion templates/repo/issue/list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
<div class="menu">
<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