From b50320a4bc53b0f2ebaa1d498287eaf88bb50582 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Thu, 21 Feb 2019 15:15:33 +0100 Subject: [PATCH 1/2] Add margin for system notification modal --- app/assets/stylesheets/system_notifications.scss | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/assets/stylesheets/system_notifications.scss b/app/assets/stylesheets/system_notifications.scss index 5fa82beaf..18c14b86f 100644 --- a/app/assets/stylesheets/system_notifications.scss +++ b/app/assets/stylesheets/system_notifications.scss @@ -111,3 +111,11 @@ margin-top: 10px; } } +#manage-module-system-notification-modal { + .modal-title { + margin-left: 20px; + } + .modal-body { + margin: 20px; + } +} From 5f78d83f9c50e14705aa61880d914826365d42a4 Mon Sep 17 00:00:00 2001 From: aignatov-bio <47317017+aignatov-bio@users.noreply.github.com> Date: Thu, 21 Feb 2019 16:15:13 +0100 Subject: [PATCH 2/2] Changing status from unread to read system notification is not correct one [SCI-3077 and SCI-3075] (#1513) * fix dropdown apperance * Remove search and fix duplicate bug * Fix specs for new method --- app/assets/javascripts/navigation.js | 19 +-------- .../javascripts/system_notifications/index.js | 28 ++++--------- .../stylesheets/system_notifications.scss | 13 ++++++ .../system_notifications_controller.rb | 3 +- app/models/user_system_notification.rb | 5 +-- .../_notification.html.erb | 41 +++++++++---------- app/views/system_notifications/index.html.erb | 2 +- config/locales/en.yml | 6 +-- .../system_notifications_controller_spec.rb | 8 ---- spec/models/user_system_notification_spec.rb | 2 +- 10 files changed, 52 insertions(+), 75 deletions(-) diff --git a/app/assets/javascripts/navigation.js b/app/assets/javascripts/navigation.js index 4e158f0fb..1bab78890 100644 --- a/app/assets/javascripts/navigation.js +++ b/app/assets/javascripts/navigation.js @@ -113,6 +113,7 @@ button .on('click', function() { noRecentText.hide(); + $('.dropdown-system-notifications .system-notification').remove(); $.ajax({ url: button.attr('data-href'), type: 'GET', @@ -120,12 +121,6 @@ beforeSend: animateSpinner($('.system-notifications-dropdown-header'), true), success: function(data) { var ul = $('.dropdown-system-notifications'); - // After closing system notification modal release system notifications dropdown - $('#manage-module-system-notification-modal').on('hidden.bs.modal', function() { - setTimeout(function() { - $('.dropdown.system-notifications')[0].dataset.closable = true; - }, 100); - }); $('.system-notifications-dropdown-header') .nextAll('.system-notification') .remove(); @@ -136,7 +131,7 @@ noRecentText.show(); } bindSystemNotificationAjax(); - SystemNotificationsMarkAsSeen('.dropdown-system-notifications'); + SystemNotificationsMarkAsSeen(); } }); $('#count-system-notifications').hide(); @@ -146,15 +141,5 @@ // init loadDropdownSystemNotifications(); - $('.dropdown-system-notifications').scroll(function() { - SystemNotificationsMarkAsSeen('.dropdown-system-notifications'); - }); loadUnseenNotificationsNumber('system-notifications', '.fa-gift'); - // Override dropdown menu closing action while system notification modal open - $('.dropdown.system-notifications').on('hide.bs.dropdown', function() { - if (this.dataset.closable === 'false') { - return false; - } - return true; - }); })(); diff --git a/app/assets/javascripts/system_notifications/index.js b/app/assets/javascripts/system_notifications/index.js index 11aaf3459..6330ebe67 100644 --- a/app/assets/javascripts/system_notifications/index.js +++ b/app/assets/javascripts/system_notifications/index.js @@ -1,18 +1,9 @@ 'use strict'; // update selected notiifcations -function SystemNotificationsMarkAsSeen(container = window) { - var WindowSize = $(container).height(); - var NotificationsToUpdate = []; - _.each($('.system-notification[data-new="1"]'), function(el) { - var NotificationTopPosition = el.getBoundingClientRect().top; - if (NotificationTopPosition > 0 && NotificationTopPosition < WindowSize) { - NotificationsToUpdate.push(el.dataset.systemNotificationId); - el.dataset.new = 0; - } - }); - if (NotificationsToUpdate.length > 0) { - $.post('/system_notifications/mark_as_seen', { notifications: JSON.stringify(NotificationsToUpdate) }); +function SystemNotificationsMarkAsSeen() { + if ($('.system-notification[data-new="1"]').length > 0) { + $.post('/system_notifications/mark_as_seen'); } } @@ -27,13 +18,15 @@ function bindSystemNotificationAjax() { $('.modal-system-notification') .on('ajax:success', function(ev, data) { - var SystemNotification = $('.system-notification[data-system-notification-id=' + data.id + ']')[0]; + var SystemNotification = $('.system-notification[data-system-notification-id=' + data.id + ']'); SystemNotificationModalBody.html(data.modal_body); SystemNotificationModalTitle.text(data.modal_title); - $('.dropdown.system-notifications')[0].dataset.closable = false; + $('.dropdown.system-notifications').removeClass('open'); // Open modal SystemNotificationModal.modal('show'); - if (SystemNotification.dataset.unread === '1') { + if (SystemNotification[0].dataset.unread === '1') { + $.each(SystemNotification, (index, e) => { e.dataset.unread = '0'; }); + SystemNotification.find('.status-icon').addClass('seen'); $.post('/system_notifications/' + data.id + '/mark_as_read'); } }); @@ -42,7 +35,7 @@ function bindSystemNotificationAjax() { function initSystemNotificationsButton() { $('.btn-more-notifications') .on('ajax:success', function(e, data) { - $(data.html).insertAfter($('.notifications-container .system-notification').last()); + $(data.html).insertAfter($('.system-notifications-container .system-notification').last()); bindSystemNotificationAjax(); if (data.more_url) { $(this).attr('href', data.more_url); @@ -52,9 +45,6 @@ function initSystemNotificationsButton() { }); } -$(window).scroll(function() { - SystemNotificationsMarkAsSeen(); -}); initSystemNotificationsButton(); SystemNotificationsMarkAsSeen(); bindSystemNotificationAjax(); diff --git a/app/assets/stylesheets/system_notifications.scss b/app/assets/stylesheets/system_notifications.scss index 18c14b86f..8cf7f8a12 100644 --- a/app/assets/stylesheets/system_notifications.scss +++ b/app/assets/stylesheets/system_notifications.scss @@ -13,6 +13,16 @@ position: relative; width: 100%; + &[data-unread="0"] { + + .body-block { + + .title { + font-weight: normal; + } + } + } + &:hover { background: $color-concrete; opacity: .7; @@ -55,6 +65,7 @@ } .title { + font-weight: bold; line-height: 20px; margin: 0; } @@ -71,9 +82,11 @@ #system-notifications-index { display: inline-block; position: relative; + width: 100%; #search-bar-notifications { border-bottom: 1px solid $color-gainsboro; + display: none; margin: 10px 0 0; padding: 0 0 10px; width: 100%; diff --git a/app/controllers/system_notifications_controller.rb b/app/controllers/system_notifications_controller.rb index 2cc7dea0c..597816dfc 100644 --- a/app/controllers/system_notifications_controller.rb +++ b/app/controllers/system_notifications_controller.rb @@ -24,8 +24,7 @@ class SystemNotificationsController < ApplicationController # Update seen_at parameter for system notifications def mark_as_seen - notifications = JSON.parse(params[:notifications]) - current_user.user_system_notifications.mark_as_seen(notifications) + current_user.user_system_notifications.mark_as_seen render json: { result: 'ok' } rescue StandardError render json: { result: 'failed' } diff --git a/app/models/user_system_notification.rb b/app/models/user_system_notification.rb index 25a6257d0..9b0704faf 100644 --- a/app/models/user_system_notification.rb +++ b/app/models/user_system_notification.rb @@ -9,9 +9,8 @@ class UserSystemNotification < ApplicationRecord scope :unseen, -> { where(seen_at: nil) } - def self.mark_as_seen(notifications_id) - where(system_notification_id: notifications_id) - .update_all(seen_at: Time.now) + def self.mark_as_seen + unseen.update_all(seen_at: Time.now) end def self.mark_as_read(notification_id) diff --git a/app/views/system_notifications/_notification.html.erb b/app/views/system_notifications/_notification.html.erb index a62e49c5d..be86a944e 100644 --- a/app/views/system_notifications/_notification.html.erb +++ b/app/views/system_notifications/_notification.html.erb @@ -1,25 +1,24 @@ -<%= link_to system_notification_path(notification.id, format: :json), class: "modal-system-notification", remote: true do %> -
-
-
"> - -
+<%= link_to system_notification_path(notification.id, format: :json), { + class: "modal-system-notification system-notification", + 'data-new': notification.seen_at ? 0 : 1, + 'data-unread': notification.read_at ? 0 : 1, + 'data-system-notification-id': notification.id, + remote: true + } do %> +
+
"> +
-
-
- <%= l(notification.last_time_changed_at, format: :full) %> -
-
- <%= notification.title %> -
-
- <%= notification.description %> -
+
+
+
+ <%= l(notification.last_time_changed_at, format: :full) %> +
+
+ <%= notification.title %> +
+
+ <%= notification.description %>
<% end %> diff --git a/app/views/system_notifications/index.html.erb b/app/views/system_notifications/index.html.erb index d4ffcee0a..058ae92bc 100644 --- a/app/views/system_notifications/index.html.erb +++ b/app/views/system_notifications/index.html.erb @@ -17,7 +17,7 @@
<% end %> -
+
<%= render partial: "list", locals: { notifications: @system_notifications[:notifications] } %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 0c0f1b15f..819d90d43 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1297,10 +1297,10 @@ en: intro_paragraph: "Hi %{user_name}, you've received What's new notification in SciNote:" index: whats_new: "What's New" - more_notifications: "More system notifications" - no_notifications: "No more system notifications" + more_notifications: "Show more notifications" + no_notifications: "No more notifications" settings: "Settings" - see_all: "See all" + see_all: "show all" activities: index: today: "Today" diff --git a/spec/controllers/system_notifications_controller_spec.rb b/spec/controllers/system_notifications_controller_spec.rb index a8507cff8..9c17f92ea 100644 --- a/spec/controllers/system_notifications_controller_spec.rb +++ b/spec/controllers/system_notifications_controller_spec.rb @@ -41,14 +41,6 @@ describe SystemNotificationsController, type: :controller do expect(user.user_system_notifications.where(seen_at: nil).count).to eq 0 end - it '#mark_as_seen response failed on wrong id\'s format' do - params = { notifications: 'wrong format' } - get :mark_as_seen, format: :json, params: params - expect(response).to have_http_status(:ok) - body = JSON.parse(response.body) - expect(body['result']).to eq 'failed' - end - it '#mark_as_read correctly set read_at' do params = { id: user.user_system_notifications.first.system_notification_id diff --git a/spec/models/user_system_notification_spec.rb b/spec/models/user_system_notification_spec.rb index ae740e5fb..d838657b5 100644 --- a/spec/models/user_system_notification_spec.rb +++ b/spec/models/user_system_notification_spec.rb @@ -55,7 +55,7 @@ describe UserSystemNotification do user: user, system_notification: notifcation_one notifications_to_update = [usn.system_notification_id] - user.user_system_notifications.mark_as_seen(notifications_to_update) + user.user_system_notifications.mark_as_seen expect(UserSystemNotification.find(usn.id).seen_at).not_to be_nil end