Some cleanup to improve readability and reduce the amount of code.
Relevant bits:
* One method name was wrong. It can actually return parameter ids,
not only top-level part ids.
* I got rid of some fail-safe checks that are never needed or moved
them to a more central place.
Change-Id: I08f2ad7bc7d3f985d6189dff170dda554f3d37c2
I moved some code around and found that quite a lot of code wants to
know:
* Is the length of this transclusion exactly 1?
* I need that 1st part.
There is more that can potentially moved from the dialog to the model.
But I don't want to make this patch to big.
Bug: T292371
Change-Id: Ia94ed0450d04dd97c4c41f5bf7c266f9a534e821
This includes some moving of code. These helper functions seem to make
more sense in the TemplateDialog class.
Bug: T292371
Change-Id: I004405bab60a569b084f9083fefa41f44f9a5561
I hope this code is a little cleaner. This patch doesn't change how
the dialog behaves.
Bug: T292210
Change-Id: I237812b3404437948eb76d8b36dcca2a4c688d6d
This method does not only select a "part", i.e. a top-level item like
a template. It also selects sub-items like template parameters. The
new name reflects this better.
Change-Id: I51a8ddbd05b283248afba5a623cc52da7b2434f5
Reasserts scroll and highlighting when toggling the sidebar, in the
case that the other panel is hidden (narrow-view mode).
FIXME: Doesn't reassert focus because the page.focus() and
bookletLayout.focus() methods don't seem to work, maybe the bug is
especially prominent when the item to focus was already marked
active.
FIXME: Stopped working for right-to-left sync on wikitext elements.
This is a less common use case and can be addressed in follow-up.
Bug: T290975
Change-Id: I94f5709e810c63ee5fd7729a192ac7b92686b88f
The .onUpdateOutlineControlButtons() method doesn't describe what it
actually does. This issue was introduced in I9c5478a. (Intentionally,
to not make the patch to complicated.) Let's continue to rename
things to be a) unique and b) honest about what they do.
This is an alternative to I8d98e61.
Bug: T289043
Change-Id: I4d52ffa6e9e5df2025a0c33031c1517bcb421279
I can't really tell what insight we get from the word "container".
Every widget is a "container" in some sense, isn't it?
This widget is just _the_ outline, I would argue.
Other suggestions?
Change-Id: I1fb27ee58c1a3dd790022504e978198dadf7ea02
This is bad for multi-template transclusions, where we focus the
first parameter of each template ending with the last. It's also
inconsistent, we don't do the same for wikitext chunks.
Change-Id: I720ce1a380a6f4a8618c3608b63557df5fb50393
This was damaging the UX by causing the first parameter to be marked
as selected, but without reliably focusing it. For example, loading
a wikitext-template-wikitext multi-part transclusion would cause the
initial focus be given to the documentation link in the template
content header. After this patch, the focus will be at the top of
the page and tab will run down window functions and then through the
sidebar.
Change-Id: I84131870ae3887dcae74d91d68c5984d1dbffd85
The weird auto-scroll feature is described in T289043#7297679.
This also fixes T291381 different than I393a2b1. Only one of the
patches should be merged.
Bug: T289043
Bug: T291381
Change-Id: I70d87f12fd68001e880510fb6c38d7c419d64b15
AddPart in the resetDialog also triggers replace part
so this call could be removed there.
Bug: T291365
Change-Id: Id6c0f5bf3aaece45da37ffab75a4d99c113944f6
This code was introduced in I8fafee6. I can't tell any more what
the "bug" mentioned in the commit message was. Let's get rid of the
duplicate code path, see if we run into regressions and deal with
them one by one. That's much easier to handle than keeping this
confusing code path around.
Note this "focusin" event handler was actually re-implementing
parts of the upstream BookletLayout, namely
OO.ui.BookletLayout.onStackLayoutFocus().
Bug: T289043
Bug: T291381
Change-Id: Ib386ae6efec08465122f0e8ee81cd6dc9a2d337a
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 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 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
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
