jenkins-bot has submitted this change and it was merged.

Change subject: Fix empty watchlist views
......................................................................


Fix empty watchlist views

Given it is array-like and there are currently instances of code which
attempt to count its contents, a MobileCollection should be Countable

Additional changes:
* Add browser tests
* Center watchlist empty illustration (looks bad on tablet)

Bug: T148599
Change-Id: Ief849fe0575407e407dd7363edef6eeec082b03b
---
M includes/models/MobileCollection.php
M resources/mobile.special.pagefeed.styles/pagefeed.less
A tests/browser/features/special_watchlist_newuser.feature
M tests/browser/features/step_definitions/common_steps.rb
M tests/browser/features/step_definitions/special_watchlist_steps.rb
M tests/browser/features/support/pages/watchlist_page.rb
6 files changed, 35 insertions(+), 4 deletions(-)

Approvals:
  Pmiazga: Looks good to me, but someone else must approve
  Bmansurov: Looks good to me, approved
  jenkins-bot: Verified

Objections:
  Florianschmidtwelzow: There's a problem with this change, please improve



diff --git a/includes/models/MobileCollection.php 
b/includes/models/MobileCollection.php
index 79931df..90f9551 100644
--- a/includes/models/MobileCollection.php
+++ b/includes/models/MobileCollection.php
@@ -7,7 +7,7 @@
 /**
  * A collection of pages, which are represented by the MobilePage class.
  */
-class MobileCollection implements IteratorAggregate {
+class MobileCollection implements IteratorAggregate, Countable {
 
        /**
         * The internal collection of pages.
@@ -17,6 +17,13 @@
        protected $pages = [];
 
        /**
+        * Return size of the collection
+        */
+       public function count() {
+               return count( $this->pages );
+       }
+
+       /**
         * Adds a page to the collection.
         *
         * @param MobilePage $page
diff --git a/resources/mobile.special.pagefeed.styles/pagefeed.less 
b/resources/mobile.special.pagefeed.styles/pagefeed.less
index 09968bf..ddb1c61 100644
--- a/resources/mobile.special.pagefeed.styles/pagefeed.less
+++ b/resources/mobile.special.pagefeed.styles/pagefeed.less
@@ -72,7 +72,7 @@
        img {
                width: 100%;
                max-width: 378px;
-               margin: 1em 0 2em;
+               margin: 1em auto 2em;
                display: block;
        }
 }
diff --git a/tests/browser/features/special_watchlist_newuser.feature 
b/tests/browser/features/special_watchlist_newuser.feature
new file mode 100644
index 0000000..82aa870
--- /dev/null
+++ b/tests/browser/features/special_watchlist_newuser.feature
@@ -0,0 +1,15 @@
+@chrome @en.m.wikipedia.beta.wmflabs.org @firefox @integration 
@test2.m.wikipedia.org @vagrant @login
+Feature: Manage Watchlist
+
+  Background:
+    Given I am using the mobile site
+      And I am logged in as a new user
+      And I am on the "Special:EditWatchlist" page
+
+  Scenario: Empty Watchlist on list view
+    Then I am informed on how to add pages to my watchlist
+
+  Scenario: Empty Watchlist on feed view
+    And I switch to the modified view of the watchlist
+    When I click the Pages tab
+    Then I am told there are no new changes
diff --git a/tests/browser/features/step_definitions/common_steps.rb 
b/tests/browser/features/step_definitions/common_steps.rb
index 6710eac..e564342 100644
--- a/tests/browser/features/step_definitions/common_steps.rb
+++ b/tests/browser/features/step_definitions/common_steps.rb
@@ -13,8 +13,6 @@
 end
 
 Given /^I am logged in as a new user$/ do
-  step 'I am on the "Main Page" page'
-  step 'I click on "Log in" in the main navigation menu'
   log_in
 end
 
diff --git a/tests/browser/features/step_definitions/special_watchlist_steps.rb 
b/tests/browser/features/step_definitions/special_watchlist_steps.rb
index 7a10cd8..3530f9c 100644
--- a/tests/browser/features/step_definitions/special_watchlist_steps.rb
+++ b/tests/browser/features/step_definitions/special_watchlist_steps.rb
@@ -34,3 +34,12 @@
 Then(/^the modified button should be selected$/) do
   expect(on(WatchlistPage).feed_link_element.parent.element.class_name).to 
match 'is-on'
 end
+
+Then(/^I am informed on how to add pages to my watchlist$/) do
+  expect(on(WatchlistPage).empty_howto_element.when_present).to be_visible
+end
+
+Then(/^I am told there are no new changes$/) do
+  expect(on(WatchlistPage).empty_panel_element.when_present).to be_visible
+  expect(on(WatchlistPage).empty_howto_element).not_to be_visible
+end
diff --git a/tests/browser/features/support/pages/watchlist_page.rb 
b/tests/browser/features/support/pages/watchlist_page.rb
index a2ad11b..0432ca2 100644
--- a/tests/browser/features/support/pages/watchlist_page.rb
+++ b/tests/browser/features/support/pages/watchlist_page.rb
@@ -5,4 +5,6 @@
   ul(:page_list_a_to_z, css: '.page-summary-list')
   a(:pages_tab_link, text: 'Pages')
   li(:selected_pages_tab, css: '.mw-mf-watchlist-selector 
li:nth-child(2).selected')
+  div(:empty_panel, css: '.empty-page')
+  img(:empty_howto, css: '.empty-page img')
 end

-- 
To view, visit https://gerrit.wikimedia.org/r/323337
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ief849fe0575407e407dd7363edef6eeec082b03b
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Florianschmidtwelzow <florian.schmidt.stargatewis...@gmail.com>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Pmiazga <pmia...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to