Skip to content

Commit

Permalink
Redesign notifications view to be more compact
Browse files Browse the repository at this point in the history
A notification has now a more compact aspect. Specially useful on
small screens. Using icons, colors and avatars makes it easier to
understand at a glance.

- An icon represents each kind of notification: request or comment.
- Review notifications are going to be replaced by requests
  notifications with "in review" state.
- Display the state of the request clearly.
- Link to the comments box and not the comment itself.
- Display all users involved in notifications
  - In case of request's notifications, it also displays all the
    reviewers including groups, projects and packages.

Co-authored-by: David Kang <dkang@suse.com>
Co-authored-by: Eduardo Navarro <enavarro@suse.com>
  • Loading branch information
3 people committed Jul 10, 2020
1 parent adb3b4f commit 15e226a
Show file tree
Hide file tree
Showing 18 changed files with 156 additions and 44 deletions.
4 changes: 3 additions & 1 deletion src/api/app/assets/images/icons/LICENSE.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
The icons used are a mixture of the [Silk Icons set](http://www.famfamfam.com/lab/icons/silk/)
Some of the icons used are a mixture of the [Silk Icons set](http://www.famfamfam.com/lab/icons/silk/)
by Mark James licensed [CC BY 2.5](https://creativecommons.org/licenses/by/2.5/) and
[Tango Icon Library](http://tango.freedesktop.org) from the public domain and images/logos
that are all rights reserved.

`request-icon.svg` icon is MIT licensed, taken from https://github.com/primer/octicons
1 change: 1 addition & 0 deletions src/api/app/assets/images/icons/request-icon.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions src/api/app/assets/stylesheets/webui/borders.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@
.border-2 {
border-width: 2px !important;
}

// NOTE: part of responsive-ux
.border-gray-500 { border-color: $gray-500 !important; }
.border-gray-400 { border-color: $gray-400 !important; }
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
grid-area: build-results;
}

.comments {
#comments-list {
grid-area: comments;
}

Expand Down
9 changes: 9 additions & 0 deletions src/api/app/assets/stylesheets/webui/responsive_ux.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ body.responsive-ux {
@import 'responsive_ux/build-result';
@import 'responsive_ux/sponsors';
@import 'responsive_ux/tabs';
@import 'responsive_ux/notifications';

.build-results .sticky-top { top: $top-bar-height; }
#log-space-wrapper #log-info.position-sticky { top: $top-bar-height; }
Expand Down Expand Up @@ -56,6 +57,14 @@ body.responsive-ux {
#notifications-filter-desktop {
.collapse { display: block !important; }
}

.grid-container {
min-height: 6.25rem;
grid-template-columns: 1fr auto 3rem;
grid-template-areas:
"notifiable who-when action"
"content content content";
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
.border-gray-500 { border-color: $gray-500 !important; }
.navbar-dark .navbar-toggler { color: $gray-300; }
.flash-and-announcement.sticky-top { top: $top-bar-height + .25rem; }
#top_0.sticky-top { top: $top-bar-height; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
.avatars-counter, .simulated-avatar {
display: inline-block;
text-align: center;
width: 1.4375rem;
height: 1.4375rem;
line-height: 1.4rem;
cursor: default;
}

.avatars-counter { font-size: 0.6rem; }

ul.avatars{
li {
margin-right: 0;
&:not(:first-child) { margin-right: -.25rem; }
}
}

.grid-container {
display: grid;
grid-template-columns: 1fr auto;
grid-template-rows: auto;
grid-template-areas:
"notifiable notifiable"
"content content"
"who-when action";
column-gap: 0.5rem;
row-gap: 0.25rem;
align-items: start;

.notifiable { grid-area: notifiable; }
.who-when { grid-area: who-when; }
.action { grid-area: action; }
.content { grid-area: content; }
}
15 changes: 15 additions & 0 deletions src/api/app/helpers/webui/notification_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ def filter_notification_link(link_text, amount, filter_item)
end
end

def request_badge_color(state)
case state
when :review, :new
'secondary'
when :declined, :revoke
'danger'
when :superseded
'warning'
when :accepted
'success'
else
'dark'
end
end

private

def filter_css(filter_item)
Expand Down
4 changes: 4 additions & 0 deletions src/api/app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ def template_name
def unread?
!delivered?
end

def unread_date
last_seen_at || created_at
end
end

# == Schema Information
Expand Down
8 changes: 8 additions & 0 deletions src/api/app/models/review.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class NotFoundError < APIError
scope :bs_request_ids_of_involved_users, ->(user_ids) { where(user_id: user_ids).select(:bs_request_id) }

scope :declined, -> { where(state: :declined) }
scope :in_state_new, -> { where(state: :new) }

before_validation(on: :create) do
self.state = :new if self[:state].nil?
Expand Down Expand Up @@ -265,6 +266,13 @@ def create_event(params = {})
Event::ReviewWanted.create(params)
end

def reviewed_by
return User.find_by(login: by_user) if by_user
return Group.find_by(title: by_group) if by_group
return Package.find_by_project_and_name(by_project, by_package) if by_package
return Project.find_by(name: by_project) if by_project
end

private

def matches_maintainers?(user)
Expand Down
56 changes: 36 additions & 20 deletions src/api/app/presenters/notification_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,31 @@ def initialize(model)
super(@model)
end

def link_to_notification_target
def notifiable_link
case @model.event_type
when 'Event::RequestStatechange', 'Event::RequestCreate', 'Event::ReviewWanted'
Rails.application.routes.url_helpers.request_show_path(@model.notifiable.number)
{ text: "Request ##{@model.notifiable.number}",
path: Rails.application.routes.url_helpers.request_show_path(@model.notifiable.number) }
when 'Event::CommentForRequest'
Rails.application.routes.url_helpers.request_show_path(@model.notifiable.commentable.number, anchor: "comment-#{@model.notifiable_id}")
{ text: "Request ##{@model.notifiable.commentable.number}",
path: Rails.application.routes.url_helpers.request_show_path(@model.notifiable.commentable.number, anchor: 'comments-list') }
when 'Event::CommentForProject'
Rails.application.routes.url_helpers.project_show_path(@model.notifiable.commentable, anchor: "comment-#{@model.notifiable_id}")
{ text: @model.notifiable.commentable.name,
path: Rails.application.routes.url_helpers.project_show_path(@model.notifiable.commentable, anchor: 'comments-list') }
when 'Event::CommentForPackage'
Rails.application.routes.url_helpers.package_show_path(package: @model.notifiable.commentable,
project: @model.notifiable.commentable.project,
anchor: "comment-#{@model.notifiable_id}")
commentable = @model.notifiable.commentable
{ text: "#{commentable.project.name} / #{commentable.name}",
path: Rails.application.routes.url_helpers.package_show_path(package: @model.notifiable.commentable,
project: @model.notifiable.commentable.project,
anchor: 'comments-list') }
else
''
end
end

def notification_badge
case @model.event_type
when 'Event::RequestStatechange', 'Event::RequestCreate'
'Request'
when 'Event::ReviewWanted'
'Review'
when 'Event::CommentForRequest', 'Event::CommentForProject', 'Event::CommentForPackage'
'Comment'
{}
end
end

def excerpt
text = case @model.notifiable_type
when 'Request'
when 'BsRequest'
@model.notifiable.description
when 'Review'
@model.notifiable.reason
Expand All @@ -45,4 +39,26 @@ def excerpt
end
text.to_s.truncate(100)
end

def kind_of_request
return unless @model.notifiable_type == 'BsRequest'

request = @model.notifiable
return "Multiple actions for project #{request.bs_request_actions.first.target_project}" if request.bs_request_actions.size > 1

BsRequest.actions_summary(@model.event_payload)
end

def commenters
commentable = @model.notifiable.commentable
commentable.comments.where('updated_at >= ?', @model.unread_date).map(&:user).uniq
end

def avatar_objects
if @model.notifiable_type == 'Comment'
commenters
else
@model.notifiable.reviews.in_state_new.map(&:reviewed_by) + User.where(login: @model.notifiable.creator)
end
end
end
2 changes: 1 addition & 1 deletion src/api/app/views/webui/package/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
srcmd5: @srcmd5,
spider_bot: @spider_bot }
.comments
.card
.card#comments-list
%h5.card-header.text-word-break-all
Comments for
= @package.name
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/views/webui/project/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
.tab-pane.fade#inherited-packages{ role: 'tabpanel', aria: { labelledby: 'inherited-packages-tab' } }
= render partial: 'project_inherited_packages', locals: { project: @project, inherited_packages: @inherited_packages }
.comments
.card
.card#comments-list
%h5.card-header.text-word-break-all
Comments for #{@project}
%span.badge.badge-primary{ id: "comment-counter-project-#{@project.id}" }
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/views/webui/request/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
= render partial: 'sourcediff_tab', collection: @actions, as: :action, locals: { bs_request: @bs_request }
.row
.col-md-8
.card.mb-3
.card.mb-3#comments-list
= render partial: 'request_comments', locals: { comments: @comments, bs_request: @bs_request }
- if @can_handle_request || (@can_add_reviews && @my_open_reviews.any?)
.card.mb-3
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
%li.list-inline-item
- case avatar_object.class.name
- when 'User', 'Group'
= image_tag_for(avatar_object, size: 23, custom_class: 'rounded-circle bg-light border border-gray-400')
- when 'Package'
- title = "Package #{avatar_object.project}/#{avatar_object}"
%span.fa.fa-archive.text-warning.rounded-circle.bg-light.border.border-gray-400.simulated-avatar{ title: title }
- when 'Project'
%span.fa.fa-cubes.text-secondary.rounded-circle.bg-light.border.border-gray-400.simulated-avatar{ title: "Project #{avatar_object}" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
%ul.list-inline.d-flex.flex-row-reverse.avatars.m-0
- if avatar_objects.size > 6
%li.list-inline-item
- hidden = (avatar_objects.size - 6)
%span.rounded-circle.bg-light.border.border-gray-400.avatars-counter{ title: "#{hidden} more users involved" }
+#{hidden}
= render partial: 'notification_avatar', collection: avatar_objects.first(6).reverse, as: :avatar_object
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
= filter_notification_link('Read', nil, type: 'read')
.row.list-group-flush.mt-5
%h5.ml-3 Filter
= filter_notification_link('Reviews', notifications_count['Review'], type: 'reviews')
= filter_notification_link('Comments', notifications_count['Comment'], type: 'comments')
= filter_notification_link('Requests', notifications_count['BsRequest'], type: 'requests')
.row.list-group-flush.mt-5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,27 @@
.list-group.list-group-flush.mt-3
- notifications.each do |n|
- notification = NotificationPresenter.new(n)
.list-group-item.d-flex.flex-column.flex-sm-row.justify-content-sm-between
.list-group-item.px-2.grid-container
.notifiable
- if notification.notifiable_type == 'BsRequest'
= image_tag('icons/request-icon', height: 18, title: 'Request notification')
= link_to(notification.notifiable_link[:text], notification.notifiable_link[:path], class: 'mx-1 text-word-break-all')
%span.badge{ class: "badge badge-#{request_badge_color(notification.notifiable.state)}" }
= notification.notifiable.state
- else
%i.fas.fa-comments{ title: 'Comment notification' }
= link_to(notification.notifiable_link[:text], notification.notifiable_link[:path], class: 'mx-1 text-word-break-all')
.actions.ml-auto.align-self-end.align-self-md-start
- title, icon = notification.unread? ? ['Mark as "Read"', 'fa-check'] : ['Mark as "Unread"', 'fa-undo']
= link_to(my_notification_path(id: notification),
method: :put, class: 'btn btn-sm btn-outline-success px-3', title: title, remote: true) do
%i.fas{ class: "#{icon}" }
.content
.text-nowrap
%span.badge.badge-secondary= notification.notification_badge
%small.text-muted= time_ago_in_words(notification.created_at)
= link_to(n.title, notification.link_to_notification_target, class: 'text-word-break-all')
%p.text-muted= notification.excerpt
- unless params[:type] == 'read'
.actions.align-self-end.align-self-sm-start.pl-3.pt-3.pt-sm-0
= link_to(my_notification_path(id: notification, type: params[:type], project: params[:project]),
method: :put, class: 'btn btn-sm btn-outline-success px-4', title: 'Mark as "Read"', remote: true) do
%i.fas.fa-check
- unless notification.unread?
.actions.align-self-end.align-self-sm-start.pl-3.pt-3.pt-sm-0
= link_to(my_notification_path(id: notification, type: params[:type], project: params[:project]),
method: :put, class: 'btn btn-sm btn-outline-success px-4', title: 'Mark as "Unread"', remote: true) do
%i.fas.fa-undo
- if notification.kind_of_request
%p.text-word-break-all.m-0.mb-2= notification.kind_of_request
%p.d-none.d-md-block.font-weight-light= notification.excerpt
%small.who-when.d-flex.align-self-center.align-self-md-start.justify-content-start.justify-content-md-end.font-weight-light
= render partial: 'notification_avatars', locals: { avatar_objects: notification.avatar_objects }
%span.d-inline-block.text-nowrap.ml-2 #{time_ago_in_words(notification.created_at)} ago

= paginate notifications, views_prefix: 'webui', window: 2

0 comments on commit 15e226a

Please sign in to comment.