[go: up one dir, main page]

Page MenuHomePhabricator

Increase VE template dialog size
Closed, ResolvedPublic1 Estimated Story Points

Description

Background
Increase size of the complex dialog to allow for more comfortable working area and to see more content at a time.

Requirements
Desktop sizing:

  • Sidebar fixed width: 240px
  • Main area max-width: 560px
  • Height: 90% viewport

Implement behind feature flag together with:
T271800: (VE Template Dialog) Move parameter descriptions below field labels
T274554: Make sidebar responsive

Mocks and Specs

Resize -Specs1.png (642×800 px, 56 KB)

Event Timeline

Lena_WMDE renamed this task from DRAFT: Increase template dialog size to Increase VE template dialog size.Mar 10 2021, 2:12 PM
Lena_WMDE set the point value for this task to 3.Mar 10 2021, 2:14 PM

Change 684328 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Optional feature makes the transclusion dialog bigger

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

There may be a range of window widths in which non-ideal things happen with the dialog, for example we'll switch to full size at a viewport of 500px when it shouldn't happen until 560px. But I couldn't find a situation where this really damages usability, so moving forward with the patch as-is.

awight subscribed.

The existing sizes in OOUI are:

	small: {
		width: 300
	},
	medium: {
		width: 500
	},
	large: {
		width: 700
	},
	larger: {
		width: 900
	},
	full: {
		width: '100%',
		height: '100%'
	}

It would be nice to avoid introducing new sizes, so could the the template dialog just use 900px/larger?

Change 684328 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Optional feature makes the transclusion dialog bigger

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

It would be nice to avoid introducing new sizes, so could the the template dialog just use 900px/larger?

Thanks for the prompt—we're discussing internally, and I've made some screenshots of what the dialog looks like with and without the sidebar, at "larger" size.

It's worth mentioning that my implementation patch is ad-hoc and does not introduce any new sizes in that enum, for better or worse.

What the keywords "medium" and "large" originally mean is specified in OO.ui.WindowManager.

  • "medium" is { width: 500 }
  • "large" is { width: 700 }
  • "larger" is { width: 900 }

I noticed a user-facing issue: When I manually scale the browser window there is a point where the template dialog expands to fullscreen. However, this point is *not* at 560px but at 500px when the sidebar is collapsed. Same when the sidebar is visible. The point is *not* at 800px then but at 700px.

Calling .getSize() still reports "medium" and "large". It appears like certain calculations in OOUI still use the original numbers because. When you think this is odd I agree. The method getSizeProperties() should be used to get this information, right? So why is there a place that access the data in OO.ui.WindowManager directly?

The code responsible for this behavior is in OO.ui.Window.prototype.getSize(). I don't think there is a mistake anywhere. This is just the result of the way the two "width" values are manipulated here without actually manipulating the original specification in OO.ui.WindowManager.

Possible ways forward:

  • Override getSize() as well. However, not only would this duplicate quite a lot of code just to change a number. We don't know if this is everything that needs to change, or if there are more edge-cases that get confused when there is a conflict between getSizeProperties() and what's actually in the specification.
  • Actually change the values for "medium" and "large" in OO.ui.WindowManager. Open question: can this affect other dialogs?
  • Add one or two new size keywords to the map in OO.ui.WindowManager and use these. The code for this is in ve.ui.MWTransclusionDialog.prototype.setMode(), as far as I can see.
  • Don't use these custom values but stick to the existing ones. When 500 and 700 (medium and large) are to small, 700 and 900 (large and larger) might work better. We would only manipulate the height, but not the width any more.

Thanks for the detailed investigation into this issue, I had mentioned in T273971#7052327 but with my usual breeziness! What I found was that this does *not* affect the user experience negatively, since jumping into fullscreen mode happens *before* the window is too narrow. So the interaction isn't totally smooth but he practical effect seems to be helpful rather than disruptive. Or do you think this is an issue we need to fix?

Yes, this needs fixing. The behavior is confusing. Instead of 1 there are 2 breakpoints now: 1.) The width becomes flexible when the dialog starts touching the window. 2.) The height changes from 90% to 100% for no apparent reason.

Users usually don't see these breakpoints. But having 2 different ones means there is a new 3rd state between the two breakpoints: The dialog touches the window left/right, but not at the top/bottom. This looks odd. It's not a floating popup window any more, but two thin slices stacked at the top/bottom of the dialog.

True, users will probably not even notice. Still it causes noise that wasn't there before.

Yes, this needs fixing. The behavior is confusing. Instead of 1 there are 2 breakpoints now: 1.) The width becomes flexible when the dialog starts touching the window. 2.) The height changes from 90% to 100% for no apparent reason.

Thanks, this description convinces me. I'll look into some of the fixes you suggest above.

Change 685731 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [WIP] Make the window size map extendable

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

Change 685731 abandoned by Awight:

[VisualEditor/VisualEditor@master] [WIP] Make the window size map extendable

Reason:

library file is not in use. Rebasing onto mediawiki-core ooui.

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

Change 685733 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/core@master] [WIP] Make the window size map extendable

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

Change 685744 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] [WIP] Rewrite custom size hack using engineered mechanism

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

The code responsible for this behavior is in OO.ui.Window.prototype.getSize(). I don't think there is a mistake anywhere. This is just the result of the way the two "width" values are manipulated here without actually manipulating the original specification in OO.ui.WindowManager.

I've fixed this by pushing the override point deeper into the base class, now we can manipulate the size map before calculations are done. If you agree with my approach, I'll upstream...

Change 685733 abandoned by Awight:

[mediawiki/core@master] [WIP] Make the window size map extendable

Reason:

We decided to not override width, so this patch became irrelevant.

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

Change 685744 abandoned by Awight:

[mediawiki/extensions/VisualEditor@master] [WIP] Rewrite custom size hack using engineered mechanism

Reason:

We're taking a different approach, no longer need to override the width.

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

Per @ECohen_WMDE discussion, we're going to use the "large" (width: 900px) size and only override the height.

@ECohen_WMDE I'm assuming we still want the 240px fixed width for the sidebar, or?

Change 689688 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Template dialog bigger size uses "larger" preset

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

Per @ECohen_WMDE discussion, we're going to use the "large" (width: 900px) size and only override the height.

Thanks you beat me to the update! "large" width when extended (which eventually become the only way it's seen)

@ECohen_WMDE I'm assuming we still want the 240px fixed width for the sidebar, or?

Yes, let's start there. When I was adjusting in browser, it was looking good.

Great, patch uploaded :-) And I meant to say "larger", but I think this was clear, it's the 900px one.

Great, patch uploaded :-) And I meant to say "larger", but I think this was clear, it's the 900px one.

Right - good catch!

awight changed the point value for this task from 3 to 1.May 12 2021, 12:17 PM

Change 689688 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Template dialog bigger size uses "larger" preset

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

Lena_WMDE claimed this task.

@ECohen_WMDE, I noticed a change in behavior that might be unintentional. When expanding and collapsing the sidebar, the behavior is different on the live wikis and on the beta cluster.

  • On enwiki, for example: The content area does have a fixed width. Enabling the sidebar makes the dialog bigger. But the content area stays the same.
  • On the beta cluster: The dialog does have a fixed width. Enabling the sidebar reduces the width of the content area.

I believe this is a mistake.

@thiemowmde I don't think this difference will have an effect since the size difference will only be deployed once the sidebar remains open, see T284203: Deploy inline descriptions, extended sidebar and bigger dialog to small set of wikis. Current sizing is correct once the sidebar is open. I would keep this ticket closed.

Oh, I forgot about this. Yea, this issue won't be visible any more when the sidebar can't be hidden any more.

There's also task T274554 to fix this on mobile, where the sidebar can still be collapsed. The solution will be that the sidebar is the full view width.