Can't change settings because the dialog doesn't scroll
Closed, ResolvedPublicbug

Description

Steps to reproduce:

  1. Zoom in enough that you can read the page comfortably without your reading glasses.
    1. Note that Design has vision simulation goggles in the office, if you don't have your own reading glasses.
  2. Go to https://en.wikipedia.org/wiki/Special:NewPagesFeed?ores=true (New Page Patrol/default section)
  3. Choose a combination of settings that has zero (or very, very few) results. Filtering for new, Featured-class articles should be sufficient.
  4. Try to change the settings to show more/different kinds of articles.
  5. You can't, because the green button is below the scroll. The dialog box can't scroll, and the page can't scroll when all the contents show on a single screen.
  6. Oh, hey, look, it's one of those secret persistent preferences that doesn't go away if you reload the page or switch browsers, and which you can't change if you can't reach the button on your screen.
  7. Phile bug report.

This screenshot is from Chrome.

Workarounds:

  • Zoom out a lot, and reload the page, so the entire, if now illegible, dialog box is small enough to fit on the screen.
  • Click the visible boxes to some more pages to your feed, then press Tab a bunch of times and hit Return, until you happen to have the off-screen green button selected when you press Return.
Restricted Application added a project: Growth-Team. · View Herald TranscriptOct 1 2018, 5:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
SBisson changed the subtype of this task from "Task" to "Bug Report".Oct 16 2018, 1:10 PM
kaldari added a comment.EditedOct 31 2018, 10:37 PM

The way to fix this is a bit tricky. The entire pop-up dialog is being displayed (albeit half off-screen), so technically you can't just make the dialog scrollable (as there's nothing to scroll to). The proper way to fix this is probably to detect if part of the dialog is off-screen at the time it opens (see https://stackoverflow.com/questions/8897289/how-to-check-if-an-element-is-off-screen), and if so, reduce the height of the dialog so that it just exactly fits on the screen and change it to overflow: auto (so that it gets scrollbars).

kaldari triaged this task as High priority.Oct 31 2018, 10:45 PM

The way to fix this is a bit tricky. The entire pop-up dialog is being displayed (albeit half off-screen), so technically you can't just make the dialog scrollable (as there's nothing to scroll to). The proper way to fix this is probably to detect if part of the dialog is off-screen at the time it opens (see https://stackoverflow.com/questions/8897289/how-to-check-if-an-element-is-off-screen), and if so, reduce the height of the dialog so that it just exactly fits on the screen and change it to overflow: auto (so that it gets scrollbars).

We already have this logic in OOUI's ClippableElement, we might be able to use that too.

@Catrope - I knew you were going to say that ;)

What about setting min-height: 550px to .mw-body-content (or some other container of the main area)? It adds a bunch of whitespace when you have no results but the popup is always completely visible.

kostajh added a subscriber: kostajh.Nov 1 2018, 2:13 PM

I prefer @SBisson's suggested approach.

Sounds good to me as long as it doesn't screw up the floating toolbars.

Change 471053 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/PageTriage@master] Ensure the page content is tall enough to show the filters panel

https://gerrit.wikimedia.org/r/471053

SBisson claimed this task.Nov 1 2018, 6:08 PM

I saw no issues with the floating toolbar in my testing.

Change 471053 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Ensure the page content is tall enough to show the filters panel

https://gerrit.wikimedia.org/r/471053

Etonkovidova added a comment.EditedNov 1 2018, 11:53 PM

@SBisson - https://en.wikipedia.beta.wmflabs.org/wiki/Special:NewPagesFeed missing <div id="siteNotice" class="mw-body-content"><!-- CentralNotice --></div>

Otherwise, it looks fine:

Looks good to me.

Change 471235 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/PageTriage@master] Set min-height on #bodyContent instead of .mw-body-content

https://gerrit.wikimedia.org/r/471235

I see 2 other elements that can be on the page with class="mw-body-content" and we don't what to target them.

The new patch uses #bodyContent instead.

Change 471235 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Set min-height on #bodyContent instead of .mw-body-content

https://gerrit.wikimedia.org/r/471235

Etonkovidova removed SBisson as the assignee of this task.Nov 2 2018, 4:49 PM
Etonkovidova closed this task as Resolved.

Re-checked in betalabs - looks good. Added to the issues to check on the post-deployment checklist.

@Etonkovidova, @SBisson - Looks like its still broken for some reason.

SBisson reopened this task as Open.Nov 9 2018, 11:09 AM
SBisson claimed this task.
SBisson moved this task from QA to In Progress on the Growth-Team (Current Sprint) board.

@Etonkovidova, @SBisson - Looks like its still broken for some reason.

Yeah, there's a large notice on top of the feed that consumes most of the min-height that was intended for the feed.

A very quick fix would be to put something like that to site css (but I don't have permission)

.mw-special-NewPagesFeed #bodyContent {
	min-height: 750px;
}

I'll work on a more deterministic fix.

Change 472637 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/PageTriage@master] Make sure the fiters menu is completely visible

https://gerrit.wikimedia.org/r/472637

Change 472637 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Make sure the fiters menu is completely visible

https://gerrit.wikimedia.org/r/472637

Etonkovidova closed this task as Resolved.Thu, Nov 15, 9:17 PM

Re-checked in wmf.4 - looks good again.