Page MenuHomePhabricator

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

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.

Screen Shot 2018-10-01 at 10.18.24 AM.png (767×1 px, 232 KB)

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.

Event Timeline

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

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).

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.

Screen Shot 2018-11-01 at 06.55.20.png (750×947 px, 131 KB)

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

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

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

Screen Shot 2018-11-01 at 4.39.59 PM.png (758×1 px, 151 KB)

Otherwise, it looks fine:

Screen Shot 2018-11-01 at 4.54.25 PM.png (733×1 px, 155 KB)

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.

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 claimed this task.
SBisson moved this task from QA to In Progress on the Growth-Team (Sprint 0 (Growth Team)) 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

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

Change 592306 had a related patch set uploaded (by Zoranzoki21; owner: Zoranzoki21):
[operations/mediawiki-config@master] Add two domains in wgCopyUploadDomains

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

Kizule subscribed.

Oops, wrong task number... Sorry..