Improve notification settings page

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

app/assets/stylesheets/sections/dashboard.scss

@@ -67,6 +67,7 @@
 
   a {
     display: block;
+    color: #333;
   }
 
   .project-name, .group-name {

app/assets/stylesheets/sections/profile.scss

@@ -114,3 +114,14 @@
     height: 50px;
   }
 }
+
+.global-notifications-form .level-title {
+  font-size: 15px;
+  color: #333;
+  font-weight: bold;
+}
+
+.notification-icon-holder {
+  width: 20px;
+  float: left;
+}

app/helpers/notifications_helper.rb

 module NotificationsHelper
   def notification_icon(notification)
     if notification.disabled?
-      content_tag :i, nil, class: 'icon-circle cred'
+      content_tag :i, nil, class: 'icon-volume-off cred'
     elsif notification.participating?
-      content_tag :i, nil, class: 'icon-circle cblue'
+      content_tag :i, nil, class: 'icon-volume-down cblue'
     elsif notification.watch?
-      content_tag :i, nil, class: 'icon-circle cgreen'
+      content_tag :i, nil, class: 'icon-volume-up cgreen'
     else
       content_tag :i, nil, class: 'icon-circle-blank cblue'
     end

app/models/notification.rb

@@ -9,12 +9,23 @@ class Notification
 
   attr_accessor :target
 
-  def self.notification_levels
-    [N_DISABLED, N_PARTICIPATING, N_WATCH]
-  end
-
-  def self.project_notification_levels
-    [N_DISABLED, N_PARTICIPATING, N_WATCH, N_GLOBAL]
+  class << self
+    def notification_levels
+      [N_DISABLED, N_PARTICIPATING, N_WATCH]
+    end
+
+    def options_with_labels
+      {
+        disabled: N_DISABLED,
+        participating: N_PARTICIPATING,
+        watch: N_WATCH,
+        global: N_GLOBAL
+      }
+    end
+
+    def project_notification_levels
+      [N_DISABLED, N_PARTICIPATING, N_WATCH, N_GLOBAL]
+    end
   end
 
   def initialize(target)
@@ -36,4 +47,8 @@ class Notification
   def global?
     target.notification_level == N_GLOBAL
   end
+
+  def level
+    target.notification_level
+  end
 end

app/views/profiles/notifications/_settings.html.haml

 %li
-  .row
-    .col-sm-4
-      %span
-        = notification_icon(notification)
-
-        - if membership.kind_of? UsersGroup
-          = link_to membership.group.name, membership.group
-        - else
-          = link_to_project(membership.project)
-    .col-sm-8
-      = form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications' do
-        = hidden_field_tag :notification_type, type, id: dom_id(membership, 'notification_type')
-        = hidden_field_tag :notification_id, membership.id, id: dom_id(membership, 'notification_id')
-
-        = label_tag nil, class: 'radio-inline' do
-          = radio_button_tag :notification_level, Notification::N_GLOBAL, notification.global?, id: dom_id(membership, 'notification_level'), class: 'trigger-submit'
-          %span Use global setting
-
-        = label_tag nil, class: 'radio-inline' do
-          = radio_button_tag :notification_level, Notification::N_DISABLED, notification.disabled?, id: dom_id(membership, 'notification_level'), class: 'trigger-submit'
-          %span Disabled
-
-        = label_tag nil, class: 'radio-inline' do
-          = radio_button_tag :notification_level, Notification::N_PARTICIPATING, notification.participating?, id: dom_id(membership, 'notification_level'), class: 'trigger-submit'
-          %span Participating
-
-        = label_tag nil, class: 'radio-inline' do
-          = radio_button_tag :notification_level, Notification::N_WATCH, notification.watch?, id: dom_id(membership, 'notification_level'), class: 'trigger-submit'
-          %span Watch
+  %span.notification-icon-holder
+    - if notification.global?
+      = notification_icon(@notification)
+    - else
+      = notification_icon(notification)
 
+  %span.str-truncated
+    - if membership.kind_of? UsersGroup
+      = link_to membership.group.name, membership.group
+    - else
+      = link_to_project(membership.project)
+  .pull-right
+    = form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications' do
+      = hidden_field_tag :notification_type, type, id: dom_id(membership, 'notification_type')
+      = hidden_field_tag :notification_id, membership.id, id: dom_id(membership, 'notification_id')
+      = select_tag :notification_level, options_for_select(Notification.options_with_labels, notification.level), class: 'trigger-submit'

app/views/profiles/notifications/show.html.haml

@@ -3,56 +3,49 @@
 %p.light
   GitLab uses the email specified in your profile for notifications
 %hr
-.alert.alert-info
-  %p
-    %i.icon-circle.cred
-    %strong Disabled
-    – You will not get any notifications via email
-  %p
-    %i.icon-circle.cblue
-    %strong Participating
-    – You will only receive notifications from related resources (e.g. from your commits or assigned issues)
-  %p
-    %i.icon-circle.cgreen
-    %strong Watch
-    – You will receive all notifications from projects in which you participate
+= form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications form-horizontal global-notifications-form' do
+  = hidden_field_tag :notification_type, 'global'
 
-.row
-  .col-sm-4
-    %h4
-      = notification_icon(@notification)
-      Global setting
-  .col-sm-8
-    = form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications' do
-      = hidden_field_tag :notification_type, 'global'
-
-      = label_tag nil, class: 'radio-inline' do
+  = label_tag :notification_level, 'Notification level', class: 'control-label'
+  .col-sm-10
+    .radio
+      = label_tag nil, class: '' do
         = radio_button_tag :notification_level, Notification::N_DISABLED, @notification.disabled?, class: 'trigger-submit'
-        %span Disabled
+        .level-title
+          Disabled
+        %p You will not get any notifications via email
 
-      = label_tag nil, class: 'radio-inline' do
+    .radio
+      = label_tag nil, class: '' do
         = radio_button_tag :notification_level, Notification::N_PARTICIPATING, @notification.participating?, class: 'trigger-submit'
-        %span Participating
+        .level-title
+          Participating
+        %p You will only receive notifications from related resources (e.g. from your commits or assigned issues)
 
-      = label_tag nil, class: 'radio-inline' do
+    .radio
+      = label_tag nil, class: '' do
         = radio_button_tag :notification_level, Notification::N_WATCH, @notification.watch?, class: 'trigger-submit'
-        %span Watch
+        .level-title
+          Watch
+        %p You will receive all notifications from projects in which you participate
 
-%br
-= link_to '#', class: 'js-toggle-visibility-link' do
-  %span.btn.btn-tiny
-    %i.icon-chevron-down
-    %span Advanced notifications settings
-.js-toggle-visibility-container.hide
+.clearfix
   %hr
-  %h4 Groups:
-  %ul.bordered-list
-    - @users_groups.each do |users_group|
-      - notification = Notification.new(users_group)
-      = render 'settings', type: 'group', membership: users_group, notification: notification
+  %p
+    You can also specify notification level per group or per project
+    %br
+    By default all projects and groups uses notification level set above
+.row.all-notifications
+  .col-md-6
+    %h4 Groups:
+    %ul.bordered-list
+      - @users_groups.each do |users_group|
+        - notification = Notification.new(users_group)
+        = render 'settings', type: 'group', membership: users_group, notification: notification
 
-  %h4 Projects:
-  %ul.bordered-list
-    - @users_projects.each do |users_project|
-      - notification = Notification.new(users_project)
-      = render 'settings', type: 'project', membership: users_project, notification: notification
+  .col-md-6
+    %h4 Projects:
+    %ul.bordered-list
+      - @users_projects.each do |users_project|
+        - notification = Notification.new(users_project)
+        = render 'settings', type: 'project', membership: users_project, notification: notification

features/steps/profile/profile_notifications.rb

@@ -8,6 +8,5 @@ class ProfileNotifications < Spinach::FeatureSteps
 
   step 'I should see global notifications settings' do
     page.should have_content "Notifications settings"
-    page.should have_content "Global setting"
   end
 end