Currently, the insert template dialog includes a back button in the
upper corner. Confirmation of abandoning unsaved changes was
accomplished in an overlay panel. This patch rewrites as a dialog
and updates the on-screen text.
Bug: T297792
Change-Id: Ifa2ff97c9284609ee2a784f455789c56a762ba50
Prevents accidentally treating plain text or user input
as HTML, which could be an XSS vulnerability.
Change-Id: Id4af48447a0907962a57340cb60aca08df9cc505
* Fix incorrect use of .append() instead of .text() (which was causing
some l10n messages to be treated as raw HTML)
* Avoid escaping and parsing HTML several times when plain text was
intended
* Remove some unused options and variables
Follow-up to 839b64d882.
Change-Id: I124257c73fe09713afefccdec8e90200e6ae433d
This sorting algorithm was introduced via Ic6bc348 (T274544). Note
there is no index parameter in the .onReplacePart() handler at this
point. When a part was moved, it was removed and simply appended
to the end. The additional sorting was needed to move it back to the
correct position.
This changed a few days later via Iafe29f1. There is now an index
parameter. The .onReplacePart() handler does the same as before, but
puts the part at the correct position right away. The additional
sorting is pointless since then.
The removed code alone is responsible for 1/3 of the total blocking
time when the template dialog opens.
Bug: T296335
Change-Id: I6c3fa70b532d34cd29d59c3b48ab81ebf608d548
onReplacePart is being called when templates are being moved up or down.
To prevent that the multipart message gets moved around passively e.g.
by one element being added above it, this patch adds it to the top after each movement.
Bug: T292829
Change-Id: I946c9bc4ba5e1d261aefbb28a8c642bb58964842
We forgot change the way that message is build while adding the link
in 07f105fd7. Now it gets parsed correctly and the link will open in
a new tab.
Bug: T284985
Change-Id: I1ed9dfdafd08d08c5aff45f4b74c540b35ec14a1
There was a remaining issue when the window was made very narrow in
desktop mode (smaller than 500px). This patch doesn't aim to really
"fix" the dialog's design in this case. The goal is to make the popup
window appear less broken, so the text can stil be read and the
buttons clicked. That's all.
This patch should not have any effect in:
a) mobile mode,
b) desktop mode when the window is wider than 500px.
Bug: T294839
Change-Id: I3171dbb991533b91eaadba63b78d0ff40aa486dc
Same as I980b72c, but with another shortcut. Again. We tried
Ctrl+Shift+X before (conflicts with RTL feature in VisualEditor), as
well as Ctrl+Alt+D (conflicts with "hide all windows" incUbuntu).
Bug: T294905
Change-Id: Iae7ba759fcd7c107ef586bd5d9ae3cdbe445cedc
For example, the Citoid extension adds a "change reference type"
button which should be visible when editing a citation template.
TODO: Decide whether we want to hide the "hide options" button in
this case. This should be handled in a separate patch and possibly
in a follow-up phase, it requires deeper changes to the logic.
Bug: T294351
Change-Id: I1c6c322fe48044d7e726bf20ba7cd2eda422cd8b
I can't reproduce this error, but I feel better having this safe
guard in place. In theory it's possible to trigger resize events
before the TransclusionModel is initialized.
Change-Id: I4bbac0f73873813629ff854ee728465c6e2a4ba7
I6909b3f0b2c153b7ee9995441e995ffa793eab40 was rebased, but
I0226ca7d39e04a69617c0d8a5d3c293cfc9e0709 was merged in the meantime.
Bug: T293202
Change-Id: I3cfec9ebc135eaf998a0982f458cfe75bfb2f01c
This is a more radical change, compared to the previous patch.
I will post more detailled explanations as comments on Gerrit.
Change-Id: I6909b3f0b2c153b7ee9995441e995ffa793eab40
Removing the selected item causes StackLayout to select (and scroll
to) its first item. To prevent this, we preemptively unselect.
Note that even when an unselected item is removed, StackLayout still
clears the selection, so this patch doesn't lose any useful
behaviors.
Fix should be pushed down into OOUI, unless there's a use case where
we want to select the first item?
Bug: T293635
Change-Id: I0c1fddfa32b76621a9f1328c8173f0158386aee8
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
This patch fixes two issues:
* The bottom corner of the new sidebar was never correct because of
a `padding: 2px` that was introduced later, but never compensated
for.
* The moment the toolbar is shown it's not a single-template dialog
any more. This implies minor style changes.
Bug: T290262
Bug: T292727
Change-Id: I08da73880c469085994ee4beb3fcdd973f80ae11
The idea of this piece of code is to make sure both sides of the
dialog show the same element. But it doesn't make sense to force the
*header* of a template into view when I clicked on a *parameter*.
Bug: T292718
Change-Id: I9945f8e54c856152f05bf717e43468ab5ab24d2f
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
As a reminder (not part of this patch): Pressing enter on the name of
a template should select it, and jump to the content area on the right.
Pressing space (that's what this patch is about) should select as well,
but not move the focus.
The best way to test the behavior is with a multi-part template.
Bug: T285323
Bug: T289043
Change-Id: I97d77f43b231696f92ba6758a6b8feac34e02e6d
* The template model fires an "add" event. Listeners don't
automatically steal the focus any more.
* Instead there is a separate "focusTemplateParameterById" event
fired from all relevant places that add a parameter.
* The "remove" doesn't steal the focus any more.
Bug: T285323
Change-Id: I93f17727524bfbcf6f11647a6c2441781337c4cc
In JavaScript .split() behaves different, compared to PHP. In
PHP the last element contains the rest of the string. In
JavaScript the rest of the string is discarded. The limit acts
as if the array is truncated. That's why we can reduce the
number in
'foo/bar'.split( '/', 1 )[ 0 ]
to 1, as we are only interested in the element "foo".
The same code in the other class is currently not covered by a
test. But changing it accordingly should be obviously fine now.
Change-Id: I20c27d480ddb1799df9eb1e5bc119b724e80653d
Pure cleanup, doesn't change behavior.
Change If8da5ae85dff63c34 included in OOUI v0.42.0 tracks invisible
controls, so it's no longer necessary to maintain persistent class
variables pointing to the buttons.
Also simplify repeated logic to make it clearly exclusive.
Bug: T290554
Change-Id: If9b6404d7061999540515645fa8e50b9a21f5a21
Previously, when the tranclusion dialog was being resized, there was
a specific width at which both the sidebar was collapsed and the dialog
was downsized from "larger/large" to "medium". This resulted in the
dialog switching from fullscreen to floating, since the dialog's width
breakpoint for "medium" doesn't match our width breakpoint for mobile.
If the user continued to downsize past this breakpoint, then the dialog
would eventually switch back to fullscreen resulting in this weird
behavior.
A simple way to prevent this is to avoid changing the dialog's size
at all. Since we don't support collapsing the new sidebar (unless
already in fullscreen), we can just leave the dialog at the
"larger/large" size.
Bug: T274554
Change-Id: I5460cdfb1a7ed73fe7957745ba37055c5f66dce1
I tried hard to come up with the best possible names. Some of the
criteria I used:
* Longer and more unique is better. This makes it much easier to
e.g. search for the event name.
* The term "part" should only be used for top-level parts. While
template parameters have a unique id, they are not a subclass
of …TransclusionPartModel and therefor not "parts".
* BookletLayout manages "pages" via "page names".
* The page names of top-level parts are identical with the part
id, see ve.ui.MWTemplateDialog.getPageFromPart.
* The page names of parameters are identical with the parameter
model id, see ve.ui.MWTemplateDialog.onAddParameter.
Some code knows parameter ids, but not what pages are. Other code
knows page names, but not what parameters are. The transition
currently happens in the …OutlineContainerWidget. We might want
to move this point up to the …TemplateDialog. But I would argue
this is good enough for now and can be changed later, if needed.
Bug: T285323
Change-Id: Iab2805b3203988db400b67c8d00e48905fdc53dc
Note there are currently two different code paths utilizing two
different events. The existing event handler actually changes
the selection of the top-level part in the sidebar (the
corresponding template name turns blue). The new event handler
highlights a parameter (it turns gray). This is currently
intentional (partly because of a bug in OOUI). I will try to
merge these code paths, if possible.
Please test, and if it works fine from the user's perspective,
please merge it as it is for now.
Bug: T285323
Bug: T289043
Change-Id: I8fafee68b8b7ff225c7b3c327f483f3426d8129c
This makes sure the corresponding top-leve part is selected in
the list on the left when navigating the main area on the
right.
Bug: T289043
Change-Id: Id1b398e1786c4099d5b14fe88dd21a106269096b
This comes with a few significant changes:
* A whole bunch of places in the code that focus and highlight
an element in the old sidebar consider the new sidebar now.
* Same when e.g. the toolbar at the bottom needs to know which
part is selected. This is read from the new sidebar now.
* To make this possible I had to merge the small helper class
we introduced in I7bc73cc back into the dialog.
It's helpful to understand how the event flow works:
* You click a template name. This does nothing (does not select
the element). It only triggers an event.
* The event is catched by the outer container that manages
all parts. From there all elements are unselected, and one
selected. This call is internal and should not trigger
another event.
Bug: T285323
Bug: T288827
Bug: T289043
Change-Id: I4a2d2b83cf2691423d4b0e6f4487228fa3c7b56d
This replaces I8cf9ecd.
Significant changes:
* The …OutlineContainerWidget doesn't need to know the
BookletLayout any more. The only remaining resason to have
this dependency was some focus management. This is now done
via an event.
* Renamed an existing event to match the new one. The two
really mean and do the exact same, even if they are
triggered from two different places.
* Simplified some existing code.
* Updated documentation.
Bug: T288827
Change-Id: Ifcf2cadabf7fa4b8ecb72e3937003fab3b00d9bb
Names like "fetch" or "resolve" are heavily ambiguous and
continue to confuse me. I hope these new names reflect better
what's going on.
Bug: T288827
Depends-On: I587a203a9370e4742f87586b4f1867b37459c375
Change-Id: I8fa47ed313e7d7b2c114a5638a67c4f3c8b830f1
This merges all code-paths that re-select a part (i.e. an input
widget on the right side of the template dialog).
Note there is an edge-case that actually changes with this
patch. When a page is removed, and creating a new page fails,
there is an `if ( page )` check. Before, the behavior was that
nothing gets selected in this case. After this change the
behavior is the same as if a page was removed: the closest one
gets selected. Not only does this make more sense. The `if` is
only a fail-safe anyway and should not result in different
behavior.
Bug: T288827
Change-Id: Ibb0260587588fb51a876658b16a81c5a73371dc4
As preparation to introduce then new UI to add unknown parameters.
This is a few things:
- Merge the code paths when adding a MWTemplateModel
- Put code adding parameters to the dialog next to each other so
that preventing reselection happens around that block
- Reduce duplicated code when re-focusing after addition
- Move adding the placeholder page to the end
- Add and clean up inline documentation
Bug: T272487
Change-Id: Ic700edd42027a928a236ed11f2c257fffe994257
This removes the paramter placeholder page from all places where it's
not usefull anymore under the new sidebar.
The new UI will be re-added in follow up patches.
Bug: T272487
Change-Id: Ifc6f6f64fed1a1b23c92282e2a1bb40a7d401d72
As discussed in Ia44da16. This change avoids possibly hundreds
of events (when a template does have hundreds of parameters),
and replaces them with a single one.
Bug: T288202
Change-Id: Ic819e8c93e872b653c238f396f1f327b6a8759d2
This is really only about the methods name, but doesn't change
any behavior.
I realized we work with several different definitions of what
"empty" means. There are at least two significant definitions:
1. When a parameter's value is the empty string or identical
to the default value, the behavior of the template is the same.
It will use the default value just as if the user entered it.
The auto-value is a meaningful value in this scenario and can't
be considered equal to the empty string.
2. The context here is when the user presses the back button.
This will destroy all user input. But an auto-value is not user
input. It will appear again when the user realizes they made a
mistake. Nothing is lost.
Personally, I would not use the word "empty" to describe this
concept. Things like "containsUserProvidedValue",
"isCustomValue", "isMeaningfulValue", … come to mind. These are
all still a big vague. A "user provided" value can be identical
to the default or auto-value. "Custom" how? I went for
"containsValuableData" for now.
Bug: T274551
Change-Id: I2912a35556795c867a6b2396cbad291e947f0ed6
I came up with a new event to do this. This event is triggered
individually for each parameter. An alternative is a single
event that gets a list of visible parameters. Is this better?
What do you think?
Bug: T288202
Change-Id: Ia44da16917c28171a01aef0f1c613dcd5d3266ba
This is – for now – intentionally done in a way that can be
undone. This will still be helpful for debugging for a while.
But we need to get rid of the duplication to be able to make
this new functionality visible on the beta cluster.
Actual removal will hapen the moment we actually remove the
old toolbar. There are already tickets for this.
Bug: T286765
Change-Id: I842c3c39a55a273af20643fa8a602d2e57fb6b8c
Note that this patch alone probably does not make that much
sense. The code executed is pretty much the same. The only
difference is that the empty (!) …ContainerWidget is kept
and re-filled with what might be a completely different
template.
This is not much of a difference to before when the
container was recreated.
This change will make more sense when the container has to
manage more state, e.g. focus states. This state will
survive then.
Change-Id: Ic336d10a595e3e222741a3dc57c1d54639166b7a
Notably:
* Don't require the model in the new sidebar via dependency
injection, but connect the event handlers later. This is
relevant because we currently create the new sidebar in the
wrong spot. Removing the hard dependency allows us to split
the code and utilize initialize() and getSetupProcess()
correctly. This will be done in a following patch.
* The change event now includes the new position. This makes
it very easy to add this missing feature to the new sidebar.
Also:
* Stop triggering change events when nothing changed. These
events are expensive. They bubble all the way up to the
TransclusionModel, and to all linked
onTransclusionModelChange() handlers.
* Update event documentation to make this more visible.
Bug: T274544
Change-Id: Iafe29f18a6fed14d9c3124c9756aa840886afbbc
Clicks on the left side now focus elements on the right
side.
This patch also simplifies the …ContainerWidget constructor.
The config parameter should only be used for "OOUI things"
that are needed by subclasses and mixins. But the parameters
we have here are not "UI things".
Passing them as config passes them to classes where we don't
know what they do with it. What probably happens is that
some class keeps a reference to the entire config object,
which doesn't have a benefit and possibly blocks garbage
collection.
Bug: T274544
Change-Id: I0c0e4a1ba59dcb43141338ffe939c9c6783e000d
Before, the new sidebar was hacked in a place where it confused
the BookletLayout logic. This became visible when using the
up/down buttons to move elements in the sidebar.
This new container wraps the new and the old sidebar. It also
uses a temporary color to make it easier to see where one ends
and the other starts.
Bug: T274544
Change-Id: I4e5b40b1d1556886fc85cff9e926a02e4888f032
For example, checking if a parameter is required works just fine
for unknown parameters. They are never required. Since I16708b0
we don't need to guard the spec related methods any more.
Change-Id: Id90e4cb810dc9faca3b26f122a534f276ee31709
I rearranged this piece of code like a dozen times before I
finally understood what it actually does. This should be much
more obvious now.
The idea is:
* If no edit was made the button is always disabled.
* You can save pretty much everything, except when the
transclusion still starts with a placeholder.
* You can also click the done button when the dialog is empty.
This feels a bit odd, but was like this before. I think this
codepath is unreachable. But it probably doesn't hurt to
keep it.
Bug: T284895
Change-Id: Ic483201b64fd64f414c5b1ec4c44198b8eadb9f2
These tags don't do much, if anything. But they provide a hint
in which scope a method might be used.
Bug: T284895
Change-Id: I0b4bdd416ee89d26961c4ded4d8bbace8c57da76
This reverts commit 950a5300dc.
Reason for revert: This broke several workflows. The reason is
that MWParameterPlaceholderPage & MWParameterSearchWidget both
hold references to the MWTemplateModel. This model is not
always the same. The dialog might be the same when a template
is edited multiple times. But the model might be a new one.
From this point on the MWParameterSearchWidget pulls data from
an outdated model.
Bug: T284636
Bug: T285571
Change-Id: I7b9ea8cab8f17705ec8020f07e3732da6ba0e73c
This does not revert commit 950a5300 but applies the most
minimal hotfix I could come up with.
The reason for the breakage is that MWParameterPlaceholderPage
& MWParameterSearchWidget both hold references to the
MWTemplateModel. This model is not always the same. The dialog
might be the same when a template is edited multiple times.
But the model might be a new one. From this point on the
MWParameterSearchWidget pulls data from an outdated model.
This extra check compares this model reference and creates a
new widget when it changed.
Bug: T284636
Bug: T285571
Change-Id: Ib3eca52bbff90ffbf56a257e3984adcbe02b310b
There is a codepath where `modelPromise` is undefined and
calling `modelPromise.then()` fails. This codepath implies
that the dialog is empty and there is nothing to update. We
can just close the dialog then.
I found this while debugging the actions in this dialog.
This happens when the dialog is empty (except for a
placeholder) but you submit it anyway. This is typically
not possible as the button is supposed to be disabled.
Still I think it's a good idea to make this code less
fragile.
The relevant code was introduced in Ibc2fc66 (2016).
Change-Id: Ia6b723548456c211b944a2320949bfc23b0afa16
This makes the code more readable and easier to reason about.
The ESLint rule responsible for this code style was removed
just recently.
Notes:
* I focus on classes that are relevant for what the WMDE team
does right now.
* I merge multiple `var` keywords only when the variables are
strongly connected.
* Caching the length in a for loop makes the code hard to
read, but not really faster when it's a trivial property
access anyway.
Bug: T284895
Change-Id: I621fed61d894a83dc95f58129bbe679d82b0f5f5
This reverts parts of I678bb24.
Brief history of this code:
2014: The dialog was designed to dynamically change the title.
There was never a tooltip.
2016: A change in OOjs changed the behavior in VE. Now the initial
title shows up as a tooltip. It never updates because VE
does not know about this. The tooltip does not match the
visible title.
2021: We revert to the behavior from 2014. We achieve this by
bypassing the codepath that creates the tooltip. This is why
….title.setLabel() is used instead of ….static.title.
Bug: T276568
Change-Id: I346a904881c3a63186d6a80afdaf717688bab42a
The tooltip is useful for languages where the dialog title might get
truncated. This patch makes sure the tooltip is always the same as
the visible label.
Bug: T276568
Change-Id: I678bb243bb5ac6d1c516ee4e146f2db9ffd5afcf
This not really just a checkbox widget anymore it inherits from
FieldLayout and became something more in that direction.
Let's use a mixture of these things to make it a bit clearer.
See also comment in Ie81b84be288553343017c4aaf8691c4e266995f5
Change-Id: Iff1746a8e5e94b56eb6c27465405aaf6b74c2310
Most notably:
* Introduce variable names that explain much better what's
going on.
* Reduce nesting.
Bug: T284895
Change-Id: I793677d8107abb6354f9e19d79c4879a41c4bd93
This action was removed via Ib744b89 in 2019, see
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/491537/4/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js
Note the messages that are removed in this patch:
* …-action-insert was used for the "insert" action.
* …-action-apply was used for an "apply" action.
* …-action-cancel doesn't mention an action. Internally,
the cancel action is "".
Since Ibd740ad the actions are registered in the
FragmentDialog superclass, see
https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/491536/2/src/ui/dialogs/ve.ui.FragmentDialog.js
Note the messages. Cancel is unchanged. …-action-insert and
…-action-apply are still there, but both linked to the same
"done" action. The "apply" and "insert" actions are gone.
I.e. they are merged into a single "done" action, represented
by a single button that changes the label from "Insert" to
"Apply changes" when needed.
On top of that,
MWTransclusionDialog.updateActionSet() replaces "Apply
changes" with "Save".
Note: Other dialogs also mention an "insert" action. I didn't
look at these. These are not in the focus of our team's
current project.
Bug: T284895
Change-Id: I1d35ada3b5b2049ed20c2d940a1c065b704c978d
The "mode" button is the button that allows to expand and
collapse the dialog. It can't be collapsed when multiple
templates are edited. That's what these lines do,
disabling the button.
"Can expand" is not the correct question. It's always
possible to expand the dialog no matter what it contains.
Bug: T284895
Change-Id: I60f3060695c80bf5541ef2156be89b85a62bf91b
Introduces new widgets forming the backbone of the experimental
template dialog sidebar.
FIXME: `text-overflow: ellipsis` is not working yet, the container
styles need adjustment.
Bug: T274543
Change-Id: Ie81b84be288553343017c4aaf8691c4e266995f5
We can skip all the up and down message passing by persisting the
parameter placeholders for each template dialog. If the parameter
list is expanded then the placeholder is deleted, on being created
again it will still have state.
To test: create a transclusion with two templates, each having many
parameters. "Add more information" to add parameters, expand the
list by clicking "Show <num> more fields", then delete the parameter
placeholder using the trash cans. Try different permutations to fool
the cache or collide with another template.
This is preparation for other template sidebar dialog work.
Bug: T284636
Change-Id: I23bdd38b173114c2a9afafc7465c4beb92d25869
These don't add any knowledge but make the code harder to read
and maintain, and are an additional source of errors.
Change-Id: Ied57741a3f985e355adfddb4e75378d5c497faa9
Begin to extract the wiring between a sidebar and the content pane of
the template dialog booklet layout. Eventually, this helper class
will present a high-level interface like "addPart(id)" and will take
care of creating the outline item, content page, and connecting
events.
Start very simple, take over the "focus" method.
Bug: T284632
Change-Id: I7bc73cc4386b99d95941fc6ed88ab5bd998de014
This reflects better what the method actually does. This patch
is a direct follow-up for the renames started in Ib029fd4.
Change-Id: Ie3e87139a5c2f5ac196e0fcc02fb897fadc99177
The names of the messages keys are very confusing. The order was set
wrong during refactoring in Ib029fd48b393d2ab7d7cff6c842789e22989e944.
We should rename the keys in a follow up in sync with translatewiki.
Bug: T284649
Change-Id: I43794d80b7df7d00441cb583ca53bcab03999e65
