From 5fd1f94e85c72625527dc5a6d5aeee0301bb95b6 Mon Sep 17 00:00:00 2001 From: Suzanne Aitchison Date: Tue, 21 Dec 2021 15:42:13 +0000 Subject: [PATCH] Add comment notification test to Cypress, including a11y improvements to notification page (#15842) * changes to accessible names on notification comment box, cypress specs * add reply to comment test, delete old specs * tweak to seeds --- .../initializers/initNotifications.js | 4 ++ app/views/notifications/index.html.erb | 3 ++ .../shared/_comment_box.html.erb | 18 ++++---- .../commentNotifications.spec.js | 45 +++++++++++++++++++ spec/support/seeds/seeds_e2e.rb | 44 ++++++++++++++++++ .../notifications/notifications_page_spec.rb | 29 ------------ 6 files changed, 106 insertions(+), 37 deletions(-) create mode 100644 cypress/integration/seededFlows/notificationsFlows/commentNotifications.spec.js diff --git a/app/assets/javascripts/initializers/initNotifications.js b/app/assets/javascripts/initializers/initNotifications.js index 3b7170802..59adc9db1 100644 --- a/app/assets/javascripts/initializers/initNotifications.js +++ b/app/assets/javascripts/initializers/initNotifications.js @@ -70,6 +70,8 @@ function initReactions() { for (var i = 0; i < butts.length; i++) { var butt = butts[i]; + butt.setAttribute('aria-pressed', butt.classList.contains('reacted')); + butt.onclick = function (event) { event.preventDefault(); sendHapticMessage('medium'); @@ -79,8 +81,10 @@ function initReactions() { function successCb(response) { if (response.result === 'create') { thisButt.classList.add('reacted'); + thisButt.setAttribute('aria-pressed', true); } else { thisButt.classList.remove('reacted'); + thisButt.setAttribute('aria-pressed', false); } } diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 89579dcc9..b8479bba0 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -68,6 +68,9 @@ <%= render "articles/fitvids" %> <%= javascript_packs_with_chunks_tag "followButtons", defer: true %> + <% else %> <%= render "devise/registrations/registration_form" %> <% end %> diff --git a/app/views/notifications/shared/_comment_box.html.erb b/app/views/notifications/shared/_comment_box.html.erb index 7795d0b94..f84ad6523 100644 --- a/app/views/notifications/shared/_comment_box.html.erb +++ b/app/views/notifications/shared/_comment_box.html.erb @@ -40,23 +40,24 @@ data-reactable-id="<%= json_data["comment"]["id"] %>" data-category="like" data-reactable-type="Comment" + aria-label="<%= t("views.reactions.like.aria_label") %>" title="heart"> - <%= inline_svg_tag("small-heart.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: t("views.comments.footer.heart.icon")) %> - <%= inline_svg_tag("small-heart-filled.svg", aria: true, class: "crayons-icon reaction-icon--like reaction-icon reacted", title: t("views.comments.footer.heart.icon")) %> + <%= inline_svg_tag("small-heart.svg", aria_hidden: true, class: "crayons-icon reaction-icon not-reacted", title: t("views.comments.footer.heart.icon")) %> + <%= inline_svg_tag("small-heart-filled.svg", aria_hidden: true, class: "crayons-icon reaction-icon--like reaction-icon reacted", title: t("views.comments.footer.heart.icon")) %> <% cache "comment-box-comment-reply-#{@last_user_comment}-#{json_data['comment']['updated_at']}-#{json_data['comment']['id']}" do %> <% reply = Comment.select(:id, :user_id, :ancestry).find_by(id: json_data["comment"]["id"])&.children&.find_by(user_id: current_user.id) %> <% if reply %> - - <%= inline_svg_tag("small-comment-filled.svg", aria: true, class: "crayons-icon reaction-icon--comment reaction-icon reacted", title: t("views.comments.footer.reply.icon")) %> + "> + <%= inline_svg_tag("small-comment-filled.svg", aria_hidden: true, class: "crayons-icon reaction-icon--comment reaction-icon reacted", title: t("views.comments.footer.reply.icon")) %> <% else %> - " href="#<%= json_data["comment"]["path"] %>"> - <%= inline_svg_tag("small-comment.svg", aria: true, class: "crayons-icon reaction-icon not-reacted", title: t("views.comments.footer.reply.icon")) %> - <%= inline_svg_tag("small-comment-filled.svg", aria: true, class: "crayons-icon reaction-icon--comment reaction-icon reacted", title: t("views.comments.footer.reply.icon")) %> + " href="#<%= json_data["comment"]["path"] %>" aria-label="<%= t("views.comments.footer.reply.text") %>"> + <%= inline_svg_tag("small-comment.svg", aria_hidden: true, class: "crayons-icon reaction-icon not-reacted", title: t("views.comments.footer.reply.icon")) %> + <%= inline_svg_tag("small-comment-filled.svg", aria_hidden: true, class: "crayons-icon reaction-icon--comment reaction-icon reacted", title: t("views.comments.footer.reply.icon")) %> <% end %> @@ -74,7 +75,8 @@ <%= f.hidden_field :commentable_type, value: json_data["comment"]["commentable"]["class"]["name"] %> <%= f.hidden_field :parent_id, value: json_data["comment"]["id"] %> - <%= f.text_area :body_markdown, id: "comment-textarea-for-#{json_data['comment']['id']}", class: "crayons-textfield mb-2 resize-y", placeholder: t("views.comments.footer.reply.placeholder"), "aria-label": t("views.comments.footer.reply.aria_label"), onfocus: "handleFocus(event)" %> + <%= f.text_area :body_markdown, id: "comment-textarea-for-#{json_data['comment']['id']}", class: "comment-textarea crayons-textfield mb-2 resize-y", placeholder: t("views.comments.footer.reply.placeholder"), "aria-label": t("views.comments.footer.reply.aria_label"), + onfocus: "handleFocus(event)" %>
diff --git a/cypress/integration/seededFlows/notificationsFlows/commentNotifications.spec.js b/cypress/integration/seededFlows/notificationsFlows/commentNotifications.spec.js new file mode 100644 index 000000000..bcf770a90 --- /dev/null +++ b/cypress/integration/seededFlows/notificationsFlows/commentNotifications.spec.js @@ -0,0 +1,45 @@ +describe('Comment notifications', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/notificationsUser.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginAndVisit(user, '/notifications'); + cy.findByRole('heading', { name: 'Notifications', level: 1 }); + }); + }); + + it('Likes and unlikes a comment', () => { + cy.get('main').within(() => { + cy.findByRole('button', { name: 'Like' }).as('like'); + + // User can like a comment + cy.get('@like') + .should('have.attr', 'aria-pressed', 'false') + .click() + .should('have.attr', 'aria-pressed', 'true'); + + // User can unlike a comment + cy.get('@like').click().should('have.attr', 'aria-pressed', 'false'); + }); + }); + + it('Replies to a comment', () => { + cy.get('main').within(() => { + // Check that comment form is initially hidden + cy.findByRole('textbox', { name: 'Reply to a comment...' }).should( + 'not.exist', + ); + + // Check the textbox appears and received immediate focus + cy.findByRole('link', { name: 'Reply' }).click(); + cy.findByRole('textbox', { name: 'Reply to a comment...' }) + .should('have.focus') + .type('Example reply text'); + + cy.findByRole('button', { name: 'Submit' }).click(); + // Check the confirmation is displayed on the page + cy.findByRole('link', { name: 'Check it out' }); + }); + }); +}); diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index dc736250e..806bf6038 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -199,6 +199,7 @@ seeder.create_if_doesnt_exist(User, "email", "notifications-user@forem.local") d checked_code_of_conduct: true, checked_terms_and_conditions: true, ) + user.notification_setting.update( email_comment_notifications: false, email_follower_notifications: false, @@ -208,8 +209,51 @@ seeder.create_if_doesnt_exist(User, "email", "notifications-user@forem.local") d website_url: Faker::Internet.url, ) + # Create a follow notification to test against follow = admin_user.follows.create!(followable: user) Notification.send_new_follower_notification_without_delay(follow) + + # Create an article comment notification to test against + seeder.create_if_doesnt_exist(Article, "slug", "notification-article-slug") do + markdown = <<~MARKDOWN + --- + title: Notification article + published: true + --- + #{Faker::Hipster.paragraph(sentence_count: 2)} + #{Faker::Markdown.random} + #{Faker::Hipster.paragraph(sentence_count: 2)} + MARKDOWN + article = Article.create!( + body_markdown: markdown, + featured: true, + show_comments: true, + user_id: user.id, + slug: "notification-article-slug", + ) + + parent_comment_attributes = { + body_markdown: Faker::Hipster.paragraph(sentence_count: 1), + user_id: user.id, + commentable_id: article.id, + commentable_type: "Article" + } + + parent_comment = Comment.create!(parent_comment_attributes) + Notification.send_new_comment_notifications_without_delay(parent_comment) + + reply_comment_attributes = { + body_markdown: Faker::Hipster.paragraph(sentence_count: 1), + user_id: admin_user.id, + commentable_id: article.id, + commentable_type: "Article", + parent: parent_comment + } + + reply = Comment.create!(reply_comment_attributes) + + Notification.send_new_comment_notifications_without_delay(reply) + end end ############################################################################## diff --git a/spec/system/notifications/notifications_page_spec.rb b/spec/system/notifications/notifications_page_spec.rb index 124f7fdc2..77eaf3ba5 100644 --- a/spec/system/notifications/notifications_page_spec.rb +++ b/spec/system/notifications/notifications_page_spec.rb @@ -25,35 +25,6 @@ RSpec.describe "Notifications page", type: :system, js: true do expect(page).not_to have_css("span#notifications-number", text: "1") end - xit "allows user to interact with replies" do - sidekiq_perform_enqueued_jobs do - article = create(:article, user: alex) - comment = create(:comment, commentable: article, user: alex) - reply = create(:comment, commentable: article, user: leslie, parent: comment) - Notification.send_new_comment_notifications_without_delay(reply) - end - - visit "/notifications" - - expect(page).to have_css("div.spec-notification") - click_button("heart") - - expect(page).to have_css(".reacted") - - click_link("Reply") - - validate_reply(leslie.comments.first.id) - end - - it "allows user to follow other users back" do - follow = leslie.follow(alex) - Notification.send_new_follower_notification_without_delay(follow, is_read: true) - visit "/notifications" - expect(page).to have_css("div.spec-notification") - click_button(I18n.t("core.follow_back")) - expect(page).to have_text(I18n.t("core.following")) - end - xcontext "when user is trusted" do before do dev_user = create(:user)