Page MenuHomePhabricator

Unnecessary empty space below edit form
Open, Needs TriagePublic

Description

The edit form (with or without header and footer) is limited in height, even though the image at the side extends further down.

Example:

proofreadpage-edit-form.png (1×1 px, 1 MB)

I've like to propose that it be changed to look more like this:

proofreadpage-edit-form-fixed.png (1×1 px, 721 KB)

i.e. still constrain the total height of the edit form (in this example, it's 75vh) but stretch the edit box to fill the available space (including the header and footer when they're visible). The image is then less visible, but it can of course still be panned and zoomed — I think it feels better to not have to move the image as well as scrolling the page.

Event Timeline

Change 700316 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/ProofreadPage@master] Use flexbox for laying out the edit form and image

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

Change 700316 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Use flexbox for laying out the edit form and image

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

Why Flexbox for this?

Flexbox shines when you have content boxes that need to (re)flow the same way text does. But that's not the case for the PRP interface: it is a rigid (but resizable) grid. CSS Grid layout would seem to be far better suited.

.prp-page-container {
  display: grid;
  grid-template-columns: 55% 45%;
  grid-template-areas: 
    "header image"
    "page   image"
    "footer image";
}

The height of the image container will then always be exactly 100% of the sum of the heights of the header/content/footer fields regardless of what, if anything, constrains them.

It's true, that would have a similar effect. I'm not sure it'd be very different from the flexbox approach though. Happy to switch, if you think it's worth it.

Also, I think we still can't use CSS grid: it's not supported by Safari 9.1 for example.

Also, I think we still can't use CSS grid: it's not supported by Safari 9.1 for example.

  1. That's insane. WMF browser stats are non-public, but all versions of Safari before 10.1 have a combined global usage share of 0.39%. Safaris less than 10.1 also lack support for ES6 Generators, ES6 arrow functions, ES6 let, Shadow DOM, and CSP 2.0 just to pick a few obvious ones. The minimum OS requirement for Safari 9 is OSX El Capitan (10.11), which is also the minimum OS requirement for Safari 11, so there are no machines out there supporting Safari 9 that do not also support Safari 11. On MobileSafari the story is slightly more restrictive but we're still talking support for devices back to the iPhone 5!
  2. Graceful fallback is acceptable in Grade A. You may conceivably be able to simply apply a Grid layout to the existing layout (I think it uses floats etc. currently).
  3. Whatever works is good enough, and He Who Writes the Code is best situated to make a call like this. But based on the experience and on the design of the two layout modes I worry going with Flexbox will be needlessly complicated to implement and cause grief down the line. Flexbox just isn't designed for this use case.

Also, I think we still can't use CSS grid: it's not supported by Safari 9.1 for example.

  1. That's insane. WMF browser stats are non-public, but all versions of Safari before 10.1 have a combined global usage share of 0.39%. Safaris less than 10.1 also lack support for ES6 Generators, ES6 arrow functions, ES6 let, Shadow DOM, and CSP 2.0 just to pick a few obvious ones. The minimum OS requirement for Safari 9 is OSX El Capitan (10.11), which is also the minimum OS requirement for Safari 11, so there are no machines out there supporting Safari 9 that do not also support Safari 11. On MobileSafari the story is slightly more restrictive but we're still talking support for devices back to the iPhone 5!

The page says the compatbility matrix is based on these browser stats

  1. Graceful fallback is acceptable in Grade A. You may conceivably be able to simply apply a Grid layout to the existing layout (I think it uses floats etc. currently).
  2. Whatever works is good enough, and He Who Writes the Code is best situated to make a call like this. But based on the experience and on the design of the two layout modes I worry going with Flexbox will be needlessly complicated to implement and cause grief down the line. Flexbox just isn't designed for this use case.

Wouldn't it make more sense to go with flexbox only for now and then shifting to the grid layout as and when we are allowed to use it? Implementing a fallback with floats is going to have its own set of problems that might require even more fiddling around to fix and get working with the grid layout

Implementing a fallback with floats is going to have its own set of problems that might require even more fiddling around to fix and get working with the grid layout

The current PRP Page:-namespace editing layout is float-based. I'm saying it's conceivable, depending on the details and structure of the current layout, that you could just add display: grid (and other Grid properties) to .prp-page-container and call it a day.

But, yes, it's certainly possible that this will turn out to be a more fiddly solution than Flexbox. These are just my suggestions based on my limited perspective (but see also T209939).

  1. WMF browser stats are non-public, but all versions of Safari before 10.1 have a combined global usage share of 0.39%. Safaris less than 10.1 also lack support for ES6 Generators, ES6 arrow functions, ES6 let, Shadow DOM, and CSP 2.0 just to pick a few obvious ones.

See T178356: Raise Grade A JavaScript requirement from ES5 (2009) to ES6 (2015)

The current PRP Page:-namespace editing layout is float-based.

Not any more it's not, that's what changed in the above patch. :-)

I think we do want to restore resize behaviour though; I've created T285529 to do that.

Is there more to be cleaned up for this task? I think it's all done.

I think we do want to restore resize behaviour though;

Yes, definitely. Without resizing there will be angry Wikisourcerors with pitchforks and torches roaming the halls (cf. the last attempt to modernize this UI). Hopefully vertical resizing using draggable dividers will be sufficient, but since the current implementation allows browser-native resizing of the HTML form textareas there may be users who have incorporated that into their personal workflow.

User reporting a problem likely caused by or related to this patch on mulWS here.

It is unclear whether something is actually broken or whether the change just broke subjective expectations / personal workflow.

User on plWS reporting issues with the horizontal layout here.

This issue sounds somewhat different than the mulWS-reported issue above.

Report from enWS here that the gutter between the page text and scan image has disappeared.

I just checked (using this page) and gutter is indeed gone in both reading and edit mode. I don't recall offhand what it used to be, but there should definitely be a small space between the two. Maybe column-gap: 1ex (or .5em or whatever) on the container, or margin-left: 1ex on the image container? Something like that.

Change 701999 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/ProofreadPage@master] Add gutter between page text and image in Page NS

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

Change 701999 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Add gutter between page text and image in Page NS

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

the gutter between the page text and scan image has disappeared.

@Xover: Fixed! Thanks. I definitely thought of it and promptly forgot.

@Samwilson Does this (flexbox) change make sense at all for the vertical layout? width:100%; flex-direction:column; is essentially just normal box model flow, except we've now limited the ability to freely resize the form fields to accommodate the flexbox in horizontal layout. Is it all downside with no real upside for that case?

User reporting a problem likely caused by or related to this patch on mulWS here.

It is unclear whether something is actually broken or whether the change just broke subjective expectations / personal workflow.

From subsequent discussions it sounds like the problem originally reported on mulWS will be addressed by the planned resizing patch. For both users reporting, the issue was regarding fitting the entire text field or both header+footer on the screen at the same time.

@Samwilson Does this (flexbox) change make sense at all for the vertical layout? width:100%; flex-direction:column; is essentially just normal box model flow, except we've now limited the ability to freely resize the form fields to accommodate the flexbox in horizontal layout. Is it all downside with no real upside for that case?

width:100%; flex-direction:column; isn't the same as box-model. It basically stretches out the elements inside the container to fill all available space vertically (whereas box-model stacks everything based on their default width). Generally speaking, this patch should allow everybody to look at more of the Wikitext inside wpTextbox1 instead of having to drag the resize handler every time.

You're right, we can probably have different styles for the top/bottom and side by side layouts (although, I also wondered if that switch could be made as simple as flipping flex-direction!). I'll have a look at how to improve the horizontal layout.

As for resizing the edit form so it all fits on the screen: that was my initial patch for this task. I'm not really sure why we ever want the edit form to be so long. Personally, I'd rather constrain it to e.g. 85vh, and use the image panning function to move the image around. (Anyway, the resizing stuff is in the other ticket.)

CSS Grid might work too: change between grid-template-areas:

img
toolbar
header
body
footer

and

toolbar toolbar
header  img
body    img
footer  img

Generally speaking, this patch should allow everybody to look at more of the Wikitext inside wpTextbox1 instead of having to drag the resize handler every time.

I think the issue is that for a lot of people the priority is seeing as much (preferably all) the UI at the same time, and seeing all the text in the text field is a secondary concern. For example, one user complained at not being able to see the header and footer fields at the same time; and another that they couldn't see the toolbar and charinsert stuff at the same time as the text at the end of the main text box. Neither of these users referred to the amount of text visible at all, or indicated that scrolling the text box (or the image for that matter) was a problem.

Digressing into my personal pie-in-the-sky soapbox, I've always wanted a full screen interface for Page: namespace, where the whole web page cannot scroll. The in-page text fields would then be smaller and always scrollable, but the behaviour is then consistent and predictable and you always see all of the UI. Having the page scrollable and multiple independently scrollable in-page UI elements just makes for annoying and unpredictable behaviour where you feel like you're fighting the interface the whole time. This is exacerbated when the text field is bigger because if the text field is big enough that its scroll bars are not enabled, a scroll gesture on the field will make the web browser infer that you wanted to scroll the whole page.

If your main machine for this task is a 24"+ desktop that always fits the vertical size with room to spare you probably will never run into this, but any reasonably sized laptop or user with age-degraded vision (and page zoom enabled) will be affected; and then it'll be their personally-preferred workflow that determines their priorities and whether they consider one or the other layout a problem.

Does anyone want to not see all of the UI at the same time then? i.e. the interface that we've had for years, where the whole page has to be scrolled in order for the save button to be visible. I feel like this UI has prioritized having the whole image on the page (even if it's not all within the viewport), and I guess I'm not really sure why that's more important than having the header/body/footer all in view (which is my preference, and what a few people seem to be saying is better).

Does anyone want to not see all of the UI at the same time then?

My guess is that we'll find all sorts out there, with mutually exclusive priorities. But I really can't claim to have anything concrete to base that guess on.

I can vividly imagine that for some users, zooming and scrolling the image would be annoying, and I personally don't ever do that unless I have to. In fact, I even wrote a little toy loupe script in order to not have to zoom and scroll the image.

If this is truly priorities along a scale where each end is mutually exclusive, it may be worthwhile considering a pref toggle of some kind to let users express that priority. But that does kinda sound like overkill.

User on plWS reporting issues with the horizontal layout here.

This issue sounds somewhat different than the mulWS-reported issue above.

Based on subsequent discussion it sounds like vertical resizing should effectively resolve this issue too.

The issue as reported was multifaceted:

  1. The text box being too tall
  2. The text box being too wide
  3. The fact that the text box was taller meant it now fits all text without enabling scroll bars, which meant initiating a scroll gesture over it would suddenly scroll the whole page which is jarring and unexpected

In the latest update the user in question indicated that the width wasn't critical. Vertical resizing will address the first issue directly, and indirectly reduce the likelihood of the last happening.

Change 704212 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/ProofreadPage@master] Simply layout switching and reduce main form height

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