Commit a625757c authored by Fatih Acet's avatar Fatih Acet

Merge branch 'fix-already-selected-activity-link' into 'master'

Fix inconsistent highlighting of already selected activity nav-links

## What does this MR do?

*  Remove edge case where user could deselect an activity nav-link (which seems to be returning all the events)
*  Explicitly add an `All` tab to return all the events

## Are there points in the code the reviewer needs to double check?
Shouldn't be

## Why was this MR needed?
Resolves existing UI inconsistency

## Screenshots (if relevant)
Before:
![4OzkoQVJYc](/uploads/fd2a7fdbde2159e875482ec7b828fe60/4OzkoQVJYc.gif)

After:
![E0lj8UhEUU](/uploads/7eb5155861eb79d72957de04c9f172c9/E0lj8UhEUU.gif)

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?

*  Closes #21631
*  Closes #21452


See merge request !6091
parents 567bcf14 b4d614bd
......@@ -29,6 +29,7 @@ v 8.13.0 (unreleased)
- Added soft wrap button to repository file/blob editor
- Add word-wrap to issue title on issue and milestone boards (ClemMakesApps)
- Fix todos page mobile viewport layout (ClemMakesApps)
- Fix inconsistent highlighting of already selected activity nav-links (ClemMakesApps)
- Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison)
- Close open merge request without source project (Katarzyna Kobierska Ula Budziszewska)
- Fix that manual jobs would no longer block jobs in the next stage. !6604
......
......@@ -21,16 +21,14 @@
};
Activities.prototype.toggleFilter = function(sender) {
var event_filters, filter;
var filter = sender.attr("id").split("_")[0];
$('.event-filter .active').removeClass("active");
event_filters = $.cookie("event_filter");
filter = sender.attr("id").split("_")[0];
$.cookie("event_filter", (event_filters !== filter ? filter : ""), {
$.cookie("event_filter", filter, {
path: gon.relative_url_root || '/'
});
if (event_filters !== filter) {
return sender.closest('li').toggleClass("active");
}
sender.closest('li').toggleClass("active");
};
return Activities;
......
......@@ -173,7 +173,8 @@ class ApplicationController < ActionController::Base
end
def event_filter
filters = cookies['event_filter'].split(',') if cookies['event_filter'].present?
# Split using comma to maintain backward compatibility Ex/ "filter1,filter2"
filters = cookies['event_filter'].split(',')[0] if cookies['event_filter'].present?
@event_filter ||= EventFilter.new(filters)
end
......
%ul.nav-links.event-filter.scrolling-tabs
= event_filter_link EventFilter.all, 'All'
= event_filter_link EventFilter.push, 'Push events'
= event_filter_link EventFilter.merged, 'Merge events'
= event_filter_link EventFilter.comments, 'Comments'
......
......@@ -2,8 +2,8 @@ class EventFilter
attr_accessor :params
class << self
def default_filter
%w{ push issues merge_requests team}
def all
'all'
end
def push
......@@ -35,18 +35,21 @@ class EventFilter
return events unless params.present?
filter = params.dup
actions = []
actions << Event::PUSHED if filter.include? 'push'
actions << Event::MERGED if filter.include? 'merged'
if filter.include? 'team'
actions << Event::JOINED
actions << Event::LEFT
case filter
when EventFilter.push
actions = [Event::PUSHED]
when EventFilter.merged
actions = [Event::MERGED]
when EventFilter.comments
actions = [Event::COMMENTED]
when EventFilter.team
actions = [Event::JOINED, Event::LEFT]
when EventFilter.all
actions = [Event::PUSHED, Event::MERGED, Event::COMMENTED, Event::JOINED, Event::LEFT]
end
actions << Event::COMMENTED if filter.include? 'comments'
events.where(action: actions)
end
......
/*= require jquery.cookie.js */
/*= require jquery.endless-scroll.js */
/*= require pager */
/*= require activities */
(() => {
window.gon || (window.gon = {});
const fixtureTemplate = 'event_filter.html';
const filters = [
{
id: 'all',
}, {
id: 'push',
name: 'push events',
}, {
id: 'merged',
name: 'merge events',
}, {
id: 'comments',
},{
id: 'team',
}];
function getEventName(index) {
let filter = filters[index];
return filter.hasOwnProperty('name') ? filter.name : filter.id;
}
function getSelector(index) {
let filter = filters[index];
return `#${filter.id}_event_filter`
}
describe('Activities', () => {
beforeEach(() => {
fixture.load(fixtureTemplate);
new Activities();
});
for(let i = 0; i < filters.length; i++) {
((i) => {
describe(`when selecting ${getEventName(i)}`, () => {
beforeEach(() => {
$(getSelector(i)).click();
});
for(let x = 0; x < filters.length; x++) {
((x) => {
let shouldHighlight = i === x;
let testName = shouldHighlight ? 'should highlight' : 'should not highlight';
it(`${testName} ${getEventName(x)}`, () => {
expect($(getSelector(x)).parent().hasClass('active')).toEqual(shouldHighlight);
});
})(x);
}
});
})(i);
}
});
})();
%ul.nav-links.event-filter.scrolling-tabs
%li.active
%a.event-filter-link{ id: "all_event_filter", title: "Filter by all", href: "/dashboard/activity"}
%span
All
%li
%a.event-filter-link{ id: "push_event_filter", title: "Filter by push events", href: "/dashboard/activity"}
%span
Push events
%li
%a.event-filter-link{ id: "merged_event_filter", title: "Filter by merge events", href: "/dashboard/activity"}
%span
Merge events
%li
%a.event-filter-link{ id: "comments_event_filter", title: "Filter by comments", href: "/dashboard/activity"}
%span
Comments
%li
%a.event-filter-link{ id: "team_event_filter", title: "Filter by team", href: "/dashboard/activity"}
%span
Team
\ No newline at end of file
require 'spec_helper'
describe EventFilter, lib: true do
describe '#apply_filter' do
let(:source_user) { create(:user) }
let!(:public_project) { create(:project, :public) }
let!(:push_event) { create(:event, action: Event::PUSHED, project: public_project, target: public_project, author: source_user) }
let!(:merged_event) { create(:event, action: Event::MERGED, project: public_project, target: public_project, author: source_user) }
let!(:comments_event) { create(:event, action: Event::COMMENTED, project: public_project, target: public_project, author: source_user) }
let!(:joined_event) { create(:event, action: Event::JOINED, project: public_project, target: public_project, author: source_user) }
let!(:left_event) { create(:event, action: Event::LEFT, project: public_project, target: public_project, author: source_user) }
it 'applies push filter' do
events = EventFilter.new(EventFilter.push).apply_filter(Event.all)
expect(events).to contain_exactly(push_event)
end
it 'applies merged filter' do
events = EventFilter.new(EventFilter.merged).apply_filter(Event.all)
expect(events).to contain_exactly(merged_event)
end
it 'applies comments filter' do
events = EventFilter.new(EventFilter.comments).apply_filter(Event.all)
expect(events).to contain_exactly(comments_event)
end
it 'applies team filter' do
events = EventFilter.new(EventFilter.team).apply_filter(Event.all)
expect(events).to contain_exactly(joined_event, left_event)
end
it 'applies all filter' do
events = EventFilter.new(EventFilter.all).apply_filter(Event.all)
expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event)
end
it 'applies no filter' do
events = EventFilter.new(nil).apply_filter(Event.all)
expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event)
end
it 'applies unknown filter' do
events = EventFilter.new('').apply_filter(Event.all)
expect(events).to contain_exactly(push_event, merged_event, comments_event, joined_event, left_event)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment