From 7fd843cd890d53c7f6df546fdf344c6e9521a084 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Mon, 25 Feb 2019 12:52:18 -0800 Subject: [PATCH] Fix talk overlay workflow The talk overlay must subscribe to the creation of new topics so that the list of topics in the talk overlay contains the newly created topic. It does this by subscribing to the talk-discussion-added event and forcing a route refresh when that has completed. Additional changes to browser tests: 1) QA: CSS selector changed for talk overlay Since I42fd7b08c4b9d92dee549d06de8a0012ea037d28 the '.add' class was removed from the talk button. This makes the browser test fail but is a false positive. 2) One of the browser tests was using the same selector to mean two different elements - the add discussion button in the talk overlay is now clearly distinguish from the "add discussion" button that is blue and appears at the bottom of talk pages Change-Id: I935b3c5f37baf242c06585ae0e2f13d059b9c324 --- resources/skins.minerva.talk/init.js | 19 ++++++++++++++++--- .../features/step_definitions/talk_steps.rb | 5 +++++ .../features/support/pages/article_page.rb | 3 ++- tests/browser/features/talk.feature | 3 +-- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/resources/skins.minerva.talk/init.js b/resources/skins.minerva.talk/init.js index 6140195..0bc9cdd 100644 --- a/resources/skins.minerva.talk/init.js +++ b/resources/skins.minerva.talk/init.js @@ -5,6 +5,8 @@ loadingOverlay = mobile.loadingOverlay, eventBus = new EventEmitter(), PageGateway = mobile.PageGateway, + api = new mw.Api(), + gateway = new PageGateway( api ), // eslint-disable-next-line jquery/no-global-selector $talk = $( '.talk, [rel="discussion"]' ), // use the plain return value here - T128273 @@ -44,8 +46,6 @@ overlayManager.add( /^\/talk\/?(.*)$/, function ( id ) { var title = talkTitle.toText(), - api = new mw.Api(), - gateway = new PageGateway( api ), talkOptions = { api: api, title: title, @@ -83,10 +83,23 @@ } return false; } ); + // After adding a new topic, we need to force a refresh of the talk topics + eventBus.on( 'talk-discussion-added', function () { + // a setTimeout is necessary since talk-discussion-added is fired + // BEFORE the overlay is closed. (FIXME) + window.setTimeout( function () { + // Force a change in the address bar + // This is important is #/talk is the current route + // (e.g. as is the case after the add discussion overlay has closed) + overlayManager.router.navigate( '#/talk/', true ); + // We use second parameter to turn on replaceState + // this ensure nobody knows above the route change above! + overlayManager.router.navigate( '#/talk', true ); + }, 300 ); + } ); } init(); - if ( inTalkNamespace ) { // reload the page after the new discussion was added eventBus.on( 'talk-added-wo-overlay', function () { diff --git a/tests/browser/features/step_definitions/talk_steps.rb b/tests/browser/features/step_definitions/talk_steps.rb index 831d373..b87f633 100644 --- a/tests/browser/features/step_definitions/talk_steps.rb +++ b/tests/browser/features/step_definitions/talk_steps.rb @@ -47,3 +47,8 @@ Then(/^there should be an add discussion button$/) do # give overlay time to fully load expect(on(ArticlePage).talkadd_element.when_present(10)).to be_visible end + +Then(/^there should be a save discussion button$/) do + # give overlay time to fully load + expect(on(ArticlePage).talktopic_save_element.when_present(10)).to be_visible +end diff --git a/tests/browser/features/support/pages/article_page.rb b/tests/browser/features/support/pages/article_page.rb index 004efb3..7502571 100644 --- a/tests/browser/features/support/pages/article_page.rb +++ b/tests/browser/features/support/pages/article_page.rb @@ -191,7 +191,8 @@ class ArticlePage div(:error_message, css: '.error') # talk overlay - a(:talkadd, css: '.add.continue') + a(:talkadd, css: '.talk-overlay .continue') + a(:talktopic_save, css: '.overlay .confirm-save') p(:talk_overlay_content_header, css: '.talk-overlay .content-header') li(:talk_overlay_first_topic_title, css: '.talk-overlay .topic-title-list li:first-child') text_field(:talk_overlay_summary, css: '.talk-overlay .summary') diff --git a/tests/browser/features/talk.feature b/tests/browser/features/talk.feature index fd9cefa..24fb59a 100644 --- a/tests/browser/features/talk.feature +++ b/tests/browser/features/talk.feature @@ -32,9 +32,8 @@ Feature: Talk Given the page "Talk:Selenium talk test" exists And I am logged into the mobile website And I am on the "Talk:Selenium UI test" page - And there should be an add discussion button When I click the talk button - Then there should be an add discussion button + Then there should be a save discussion button @integration Scenario: A newly created topic appears in the list of topics immediately