Page MenuHomePhabricator

QA of 'specialpages' feature branch (settings page)
Closed, ResolvedPublic

Assigned To
Authored By
ovasileva
Jan 11 2018, 4:49 PM
Referenced Files
F12516848: IMG_1731.PNG
Jan 12 2018, 8:23 PM
F12516834: IMG_1730.PNG
Jan 12 2018, 8:23 PM
F12516833: IMG_1728.PNG
Jan 12 2018, 8:23 PM
F12516832: IMG_1727.PNG
Jan 12 2018, 8:23 PM
F12516835: IMG_1729.PNG
Jan 12 2018, 8:23 PM

Description

Background

We developed the specialpages feature branch as a series of separate tasks, but decided to do one full round of QA prior to deploying the entire branch. This QA round will account for all subtasks of T182217: Deploy `specialpages` feature branch (promote new Special:MobileOptions, fontchanger etc)

Acceptance criteria

  • Complete all test cases for the specialpages feature branch
  • For each bug found, provide description and screenshots

Event Timeline

ovasileva triaged this task as Medium priority.Jan 11 2018, 4:49 PM
ovasileva created this task.
ovasileva renamed this task from QA of 'specialpages' feature branch (settings page_ to QA of 'specialpages' feature branch (settings page).Jan 11 2018, 4:49 PM
ovasileva raised the priority of this task from Medium to High.Jan 11 2018, 4:54 PM

Here is some initial feedback from an exploratory run on these testcases. I am still working through some other devices/browsers:

Font Changer -
Works:
The settings page is loading. Font changer has a distinct portion of the settings page. Font changer has the desired options. In general, font changes are appearing in articles
Enhancements:

  1. Enhancement - It would be nice if the font size on the settings page changed as you chose a new setting to give a visual indication of what the font will be.
  2. Possible bug - I was playing with the font setting a bit and ran into a couple of issues. The back button seems to cancel font changes. For example, if the user is reading an article and they decide to change the font size, they have to do this: Tap menu. Tap settings and change font. Tap back. At this point, the font change is canceled. The only workaround is to perform a new search for the same article from the settings page. We may want to add 'done/cancel' buttons or something, or make settings changes persist through browser 'back'.
  3. Possible Bug - Font size changes do not seem to effect text in things like lists and infoboxes (some examples below)
  4. Staging Issue? - Right now tapping 'send feedback' goes to Nirzar talk page, but I assume that's just because it is on staging.
Font sizes in lists and infoboxes
IMG_1727.PNG (2×1 px, 451 KB)
IMG_1728.PNG (2×1 px, 520 KB)
IMG_1730.PNG (2×1 px, 527 KB)
IMG_1729.PNG (2×1 px, 945 KB)

Expand Sections -
Works:
Setting toggle is present and seems to work. Expand/collapse is working on articles.
Enhancements:

  1. Possible Bug - Similar to item #2 above, the back button cancels the setting change.
  2. Question - If the user has manually collapsed or expanded sections in an article and then makes the setting change, should those sections expand/collapse? It's seems like a cookie issue, but... Here is what I see right now and it 'looks' possibly correct, but want to check: Toggle expand to on, navigate to an article (Security Article on Staging) and collapse all but one section. Return to settings, toggle expand to off and search for the same article. View the sections. In this case, the one section left expanded remains expanded. Also, if toggled on, then the expanded sections appear on articles. If the user then toggles it off and returns to any of those articles all the sections are still expanded.

Beta Features -
Works:
Toggle is present, Toggling switches the below items to 'locked' or 'checked.'
Enhancements:
Once or twice I managed to get settings into a state where the check or lock icons were missing. I am not sure how to reproduce it right now.

IMG_1731.PNG (2×1 px, 400 KB)

Here is some initial feedback from an exploratory run on these testcases. I am still working through some other devices/browsers:

Font Changer -
Works:
The settings page is loading. Font changer has a distinct portion of the settings page. Font changer has the desired options. In general, font changes are appearing in articles
Enhancements:

  1. Enhancement - It would be nice if the font size on the settings page changed as you chose a new setting to give a visual indication of what the font will be.
  2. Possible bug - I was playing with the font setting a bit and ran into a couple of issues. The back button seems to cancel font changes. For example, if the user is reading an article and they decide to change the font size, they have to do this: Tap menu. Tap settings and change font. Tap back. At this point, the font change is canceled. The only workaround is to perform a new search for the same article from the settings page. We may want to add 'done/cancel' buttons or something, or make settings changes persist through browser 'back'.

@Jdlrobson , @Nirzar - did we overlook this or is it a bug? The back button shouldn't reset the changes.

  1. Possible Bug - Font size changes do not seem to effect text in things like lists and infoboxes (some examples below)
  2. Staging Issue? - Right now tapping 'send feedback' goes to Nirzar talk page, but I assume that's just because it is on staging.

I'll be setting up the appropriate page for this to go to and post in T182217: Deploy `specialpages` feature branch (promote new Special:MobileOptions, fontchanger etc)

Expand Sections -
Works:
Setting toggle is present and seems to work. Expand/collapse is working on articles.
Enhancements:

  1. Possible Bug - Similar to item #2 above, the back button cancels the setting change.
  2. Question - If the user has manually collapsed or expanded sections in an article and then makes the setting change, should those sections expand/collapse? It's seems like a cookie issue, but... Here is what I see right now and it 'looks' possibly correct, but want to check: Toggle expand to on, navigate to an article (Security Article on Staging) and collapse all but one section. Return to settings, toggle expand to off and search for the same article. View the sections. In this case, the one section left expanded remains expanded. Also, if toggled on, then the expanded sections appear on articles. If the user then toggles it off and returns to any of those articles all the sections are still expanded.

This is expected behavior

@Jdlrobson , @Nirzar - did we overlook this or is it a bug? The back button shouldn't reset the changes.

In the case of the back button many browsers will send you to the cached page rather than request it again. There's thus no way to actually force this to refresh :/
(Refresh the page and the font size should go into effect - please confirm!)

Previously we redirected to the page after submitting the form but now save is async this is no longer a possibility.

I just tried these steps on reading staging. The settings don't get reverted for me. they do get saved even with back button but you have to refresh the page to see the change take effect.

it's not ideal but there's no other way. I'm fine with this behaviour but @ABorbaWMF should confirm that even with back button it in fact saves the option but does not reflect it without a refresh

@Nirzar - Yes refreshing will reflect the settings changes.

@ABorbaWMF Thanks!

@ovasileva not a bug, but a future feature request?

@Nirzar - during standup, @Jdlrobson suggested that for the time being we move the font switcher back to beta - I've set up T185152: Depromote font switcher back to beta in specialpages feature branch, and we can make a secondary task for feature updates. Perhaps we can just have a toast informing the user to refresh?

After chatting with @Nirzar I've opened T185166 to see if there is some way we can fix this before shipping. If there is, this will be less work then moving the font switcher back into beta.

After chatting with @Nirzar I've opened T185166 to see if there is some way we can fix this before shipping. If there is, this will be less work then moving the font switcher back into beta.

Sounds good, now prioritized.

I've put the back button fix patch on staging in the hope that it helps us complete testing. It's becoming a pain to have to sync the branch so I'm keen for us to do T182217 ASAP. Let me know if that fixes the problem!

Great! @ABorbaWMF - could you re-test the back button functionality? @Jdlrobson - does T185166: [Spike, 2hrs] Can we detect back button clicks after changing font still need to go through code review? We can keep the QA on here and close both of them together.

@ovasileva I just did a quick test and it looks ok in terms of the back button. I am still seeing this one: "Font size changes do not seem to effect text in things like lists and infoboxes"

@ABorbaWMF - for lists and infoboxes, that it by design. And with that, I believe that concludes QA! Moving to signoff. Looks good to be, but I think @Jdlrobson and @Nirzar should also take a final look before closing.

Specialpages branch also has minor impact [1] on other pages. not sure if we want to test these as the change is very minor but good to have a record of it. I have checked the following special pages and they work as expected

  • Nearby
  • Log in
  • Create account
  • Uploads
  • Page history
  • User contributions
  • Changes
  • Log (deletion log)

[1] the expected change is the titles of these pages should be larger(than production) and left aligned instead of centre aligned

@ABorbaWMF - do you think you could give one of these (nearby seems to be a good candidate) a very quick look across different browsers? I'm assuming everything is fine, but let's check for the sake of thoroughness.

Anything left to do here?
https://reading-web-staging.wmflabs.org/wiki/Special:MobileOptions is running the latest code and I'm super keen to get this into master asap as the code is starting to converge and become hard to manage given other efforts on other projects e.g. download button.

Working for me so far. I am trying a few different browsers.

ovasileva updated the task description. (Show Details)

Sounds like we're done here.

238482n375 lowered the priority of this task from High to Lowest.
238482n375 moved this task from Next Up to In Code Review on the Analytics-Kanban board.
238482n375 edited subscribers, added: 238482n375; removed: Aklapper.

SG9tZVBoYWJyaWNhdG9yCk5vIG1lc3NhZ2VzLiBObyBub3RpZmljYXRpb25zLgoKICAgIFNlYXJjaAoKQ3JlYXRlIFRhc2sKTWFuaXBoZXN0ClQxOTcyODEKRml4IGZhaWxpbmcgd2VicmVxdWVzdCBob3VycyAodXBsb2FkIGFuZCB0ZXh0IDIwMTgtMDYtMTQtMTEpCk9wZW4sIE5lZWRzIFRyaWFnZVB1YmxpYwoKICAgIEVkaXQgVGFzawogICAgRWRpdCBSZWxhdGVkIFRhc2tzLi4uCiAgICBFZGl0IFJlbGF0ZWQgT2JqZWN0cy4uLgogICAgUHJvdGVjdCBhcyBzZWN1cml0eSBpc3N1ZQoKICAgIE11dGUgTm90aWZpY2F0aW9ucwogICAgQXdhcmQgVG9rZW4KICAgIEZsYWcgRm9yIExhdGVyCgpUYWdzCgogICAgQW5hbHl0aWNzLUthbmJhbiAoSW4gUHJvZ3Jlc3MpCgpTdWJzY3JpYmVycwpBa2xhcHBlciwgSkFsbGVtYW5kb3UKQXNzaWduZWQgVG8KSkFsbGVtYW5kb3UKQXV0aG9yZWQgQnkKSkFsbGVtYW5kb3UsIEZyaSwgSnVuIDE1CkRlc2NyaXB0aW9uCgpPb3ppZSBqb2JzIGhhdmUgYmVlbiBmYWlsaW5nIGF0IGxlYXN0IGEgZmV3IHRpbWVzIGVhY2guIE1vcmUgaW52ZXN0aWdhdGlvbiBuZWVkZWQuCkpBbGxlbWFuZG91IGNyZWF0ZWQgdGhpcyB0YXNrLkZyaSwgSnVuIDE1LCA3OjIxIEFNCkhlcmFsZCBhZGRlZCBhIHN1YnNjcmliZXI6IEFrbGFwcGVyLiC3IFZpZXcgSGVyYWxkIFRyYW5zY3JpcHRGcmksIEp1biAxNSwgNzoyMSBBTQpKQWxsZW1hbmRvdSBjbGFpbWVkIHRoaXMgdGFzay5GcmksIEp1biAxNSwgNzoyMiBBTQpKQWxsZW1hbmRvdSB1cGRhdGVkIHRoZSB0YXNrIGRlc2NyaXB0aW9uLiAoU2hvdyBEZXRhaWxzKQpKQWxsZW1hbmRvdSBhZGRlZCBhIHByb2plY3Q6IEFuYWx5dGljcy1LYW5iYW4uCkpBbGxlbWFuZG91IG1vdmVkIHRoaXMgdGFzayBmcm9tIE5leHQgVXAgdG8gSW4gUHJvZ3Jlc3Mgb24gdGhlIEFuYWx5dGljcy1LYW5iYW4gYm9hcmQuCkNoYW5nZSBTdWJzY3JpYmVycwpDaGFuZ2UgUHJpb3JpdHkKQXNzaWduIC8gQ2xhaW0KTW92ZSBvbiBXb3JrYm9hcmQKQ2hhbmdlIFByb2plY3QgVGFncwpBbmFseXRpY3MtS2FuYmFuCtcKU2VjdXJpdHkK1wpXaWtpbWVkaWEtVkUtQ2FtcGFpZ25zIChTMi0yMDE4KQrXClNjYXAK1wpTY2FwIChTY2FwMy1BZG9wdGlvbi1QaGFzZTIpCtcKQWJ1c2VGaWx0ZXIK1wpEYXRhLXJlbGVhc2UK1wpIYXNodGFncwrXCkxhYnNEQi1BdWRpdG9yCtcKTGFkaWVzLVRoYXQtRk9TUy1NZWRpYVdpa2kK1wpMYW5ndWFnZS0yMDE4LUFwci1KdW5lCtcKTGFuZ3VhZ2UtMjAxOC1KYW4tTWFyCtcKSEhWTQrXCkhBV2VsY29tZQrXCkJvbGQKSXRhbGljcwpNb25vc3BhY2VkCkxpbmsKQnVsbGV0ZWQgTGlzdApOdW1iZXJlZCBMaXN0CkNvZGUgQmxvY2sKUXVvdGUKVGFibGUKVXBsb2FkIEZpbGUKTWVtZQpQcmV2aWV3CkhlbHAKRnVsbHNjcmVlbiBNb2RlClBpbiBGb3JtIE9uIFNjcmVlbgoyMzg0ODJuMzc1IGFkZGVkIHByb2plY3RzOiBTZWN1cml0eSwgV2lraW1lZGlhLVZFLUNhbXBhaWducyAoUzItMjAxOCksIFNjYXAgKFNjYXAzLUFkb3B0aW9uLVBoYXNlMiksIEFidXNlRmlsdGVyLCBEYXRhLXJlbGVhc2UsIEhhc2h0YWdzLCBMYWJzREItQXVkaXRvciwgTGFkaWVzLVRoYXQtRk9TUy1NZWRpYVdpa2ksIExhbmd1YWdlLTIwMTgtQXByLUp1bmUsIExhbmd1YWdlLTIwMTgtSmFuLU1hciwgSEhWTSwgSEFXZWxjb21lLlBSRVZJRVcKMjM4NDgybjM3NSBtb3ZlZCB0aGlzIHRhc2sgZnJvbSBJbiBQcm9ncmVzcyB0byBJbiBDb2RlIFJldmlldyBvbiB0aGUgQW5hbHl0aWNzLUthbmJhbiBib2FyZC4KMjM4NDgybjM3NSByZW1vdmVkIEpBbGxlbWFuZG91IGFzIHRoZSBhc3NpZ25lZSBvZiB0aGlzIHRhc2suCjIzODQ4Mm4zNzUgdHJpYWdlZCB0aGlzIHRhc2sgYXMgTG93ZXN0IHByaW9yaXR5LgoyMzg0ODJuMzc1IHJlbW92ZWQgc3Vic2NyaWJlcnM6IEFrbGFwcGVyLCBKQWxsZW1hbmRvdS4KQ29udGVudCBsaWNlbnNlZCB1bmRlciBDcmVhdGl2ZSBDb21tb25zIEF0dHJpYnV0aW9uLVNoYXJlQWxpa2UgMy4wIChDQy1CWS1TQSkgdW5sZXNzIG90aGVyd2lzZSBub3RlZDsgY29kZSBsaWNlbnNlZCB1bmRlciBHTlUgR2VuZXJhbCBQdWJsaWMgTGljZW5zZSAoR1BMKSBvciBvdGhlciBvcGVuIHNvdXJjZSBsaWNlbnNlcy4gQnkgdXNpbmcgdGhpcyBzaXRlLCB5b3UgYWdyZWUgdG8gdGhlIFRlcm1zIG9mIFVzZSwgUHJpdmFjeSBQb2xpY3ksIGFuZCBDb2RlIG9mIENvbmR1Y3QuILcgV2lraW1lZGlhIEZvdW5kYXRpb24gtyBQcml2YWN5IFBvbGljeSC3IENvZGUgb2YgQ29uZHVjdCC3IFRlcm1zIG9mIFVzZSC3IERpc2NsYWltZXIgtyBDQy1CWS1TQSC3IEdQTApZb3VyIGJyb3dzZXIgdGltZXpvbmUgc2V0dGluZyBkaWZmZXJzIGZyb20gdGhlIHRpbWV6b25lIHNldHRpbmcgaW4geW91ciBwcm9maWxlLCBjbGljayB0byByZWNvbmNpbGUu