I was (again) wondering why the try-catch is there. But it's actually
needed. Now covered by a test.
Bug: T290140
Bug: T291062
Change-Id: Iccc7274ed9e7b81b8491dbca7d3d771706b0ed09
Note there is still an issue with the upstream
OO.ui.BookletLayout.selectFirstSelectablePage() method stealing
the focus in some situations when you press space. Still this patch
already improves the situation. Pressing space on both top-level
template elements as well as parameters should scroll the thing into
view, but keep the focus in the sidebar. This was just not happening
at all.
Make sure to use a very long multi-part template to test this.
Bug: T289043
Change-Id: I9c5478a04b14b94ccd5d00480d48a7d59b4e0c37
We broke something with the change I166b971. When we renamed that
method it started to override (and therefor disable) the method
with the same name from the base class.
I decided to move all code in the subclass for the moment. It might
be misplaced there (note how almost all code related to the new
sidebar is in the base class). But this is cleanup work for later.
Bug: T289043
Bug: T291151
Change-Id: Id255585e78967eee0f72c27727cd23211674923c
The most basic fix. The "outlined" flag is set to false for the
cite dialog, in contrast to the transclusion dialog. It's always
true for the transclusion dialog. Note it doesn't mean the sidebar
is visible, but specifies if a sidebar is created in the first
place.
Bug: T291241
Change-Id: I5a8b538949e9fd0b8e85a6a91ca2420ef72e4612
We removed this line of code in a recent patch, but it turns out
it's still necessary in at least one situation:
* Make sure you have a multi-part template where the first part is
a wikitext snippet.
* Edit the template.
* Click the very first item in the sidebar.
Nothing happens. But the text cursor should be in the wikitext
field.
Another situation:
* Put the text cursor in the first wikitext field.
* Press shift + tab. Now a button in the bottom toolbar should have
the focus.
* Click the 1st element in the sidebar. Again, nothing happens.
The extra .focus() call is redundant in many situations. But it also
doesn't hurt to repeat it. It will just re-focus the element that's
already focused.
Bug: T289043
Change-Id: Iccbe376b98a1b1e5469cd17e1c95d5d8869442d3
It was called like this because the event is literally called "set".
But it doesn't explain _what_ is set.
The new name .onBookletLayoutSetPage() already appears somewhere
else in the codebase.
Change-Id: I166b971c08f5d0fae97fc9d6244117a680f84b7c
The previous patch Id314ee8 was incomplete. The event changed.
The id in the event is not guaranteed to be a top-level partId any
more, but can be a template parameter's id.
Note: "Parameter id" and "pageName" is the same. The fact that
these ids match is how the left and the right side of the dialog
communicate.
Bug: T289043
Bug: T291151
Change-Id: I391f0f8edb96398fd33a2e0b01003013c52776da
I realised these are vital information to make the buttons at
the bottom of the template dialog behave sane. It's still
possible to focus this page, even if it doesn't have a visible
item in any of the old/new sidebars. This is when these flags
are used to decide if the up/down/remove buttons should be
enabled.
Bug: T291151
Change-Id: I6ab709b856d110bfb37daa1592c0b6a99714aa25
The OO.ui.OutlineOptionWidget class does have an .isRemovable()
state. This is how OO.ui.OutlineControlsWidget decides if the
remove button can be used.
It appears like ve.ui.MWParameterPage forgot to mark required
parameters as not removable.
This makes some oddly specific code in ve.ui.MWTransclusionDialog
obsolete. Note how this class does not contain any other code
about "required" or other flags specific to parameters.
Note that one aspect of this patch will be visible in the old
sidebar: The trashcan will be visible on required parameters, but
disabled. It was hidden before. However, this actually improves
the UX. Hiding the trashcan made the up/down buttons jump around.
This makes it unnecessary hard to hit them. It also causes visual
distraction when navigating the list of parameters.
Let's stick to the upstream OOUI behavior.
The remove button still disappears when the only element in the
dialog is the template search widget. This is clearly an
entirely different state. (Don't ask why the up/down buttons are
not hidden. It was like this before.)
Bug: T289039
Change-Id: If78881e503f19f497f1993da4e5b9b09ee538307
The separate setup method was introduced in 2014 via I7c3c133.
It appears like most of the code here was written before this
method existed. Let's update it.
* this.outlineItem is guaranteed to be set. No need for the `if`.
* The parent method is effectively abstract. There is no point in
calling it, I would argue.
* The return value is never used. I.e. this method is never
chained, and probably shouldn't.
Change-Id: Ida26ebdf09be74958936c3950ebdf6def9a69bc0
New changes:
40bf00bfc Make default source mode font easier to override
ec9e8022e Update and fix all `@param config` and `@cfg` documentation
Bug: T290176
Change-Id: I20ddb21c23ac817c500f7f854bd2bb5d88bf779c
The behavior of the enter key in the new template dialog sidebar
is somewhat inconsistent. When pressing enter on the name of a
template it sometimes just doesn't work, but focuses something
else.
I realized this is because the message "The … template doesn't
yet exist." does not have a link. There is nothing to focus in
this element. The code just gives up and the selection returns
to whatever was selected before.
It works when there is a link in the template header. But this
is not even that useful.
Let's try to always focus the first parameter instead. The user
can still press Shift + Tab to focus the link to the template
page.
Bug: T289043
Change-Id: Id314ee8ebf47d387df08c7fb432094b6d8f7a3d2
The previous patch I15aa2c0 (approved by UX) was incomplete. The
required indicator was still shown, depending on the skin.
This patch also reduces the amount of generated HTML when it
doesn't have an effect anyway. At the moment an empty <span></span>
is generated for _every_ parameter in the dialog. That's potentially
hundreds. But the element is only needed for deprecated and
(in the old UI) required parameters.
A missing space is added while we touch this code anyway. The
missing whitespace between label and indicator icon is confirmed to
be a bug by UX.
Styles that are the same on all skins are moved to the .css file
that's loaded for all skins. Missing word-wrapping for overly long
template parameter names (on the right side of the dialog) is added.
The position of the indicator icon was broken on Minerva the moment
a parameter name is a bit longer. Fixed by replacing `inline-block`
with `inline`.
Bug: T290492
Change-Id: Ie346d88969cec2effaf90d328d08567ab7b7bf75
This applies in several situations. A trivial one is a parameter
that's already in use, but you uncheck it while the relevant
error message is shown. Vice versa.
Bug: T290977
Change-Id: Ia4114194a2efe34a7d51e633c776ce892cc9cb18
This patch improves the error handling for when a user tries to add
a parameter which is either an alias of a existing parameter, the
primary name of a existing aliased parameter, or a name/alias of an
existing parameter which is shown with an override label.
The error message was modified to always refer to the conflicting
parameter using the same name that is has in the sidebar.
Example: A parameter named "Parameter B" is already present in the
sidebar under its alias "B". When a user tries to add "Parameter B",
the new error message will inform the user that the parameter they
are trying to add already exists as "B".
Bug: T285869
Change-Id: I762b72b6cf14eb8ff5fcef63b4dcb70e297050de
It's impossible to use the delete button from the
OO.ui.OutlineControlsWidget to delete a parameter when the new
sidebar is active.
This partly fixes one of the issues mentioned in I97d77f4. The
delete button might stil become active for a moment (not fixed
yet), but doesn't stay active forever any more with this fix
in place.
Apparently this also makes another workaround obsolete.
Change-Id: I0bca310772c26149170af23ff8e5505c3ce4adf4
