Commit graph

248 commits

Author SHA1 Message Date
jenkins-bot 6dc75d8283 Merge "Remove obsolete performance bottleneck from TransclusionModel" 2021-12-17 13:24:19 +00:00
Thiemo Kreuz 845cd69bf5 Don't scroll to template parameters while the dialog loads
Bug: T296743
Change-Id: Icd501ae89695a202e8c264d1b5e9811e812a56d1
2021-12-09 14:57:20 +01:00
Thiemo Kreuz fd669a4d16 Remove obsolete performance bottleneck from TransclusionModel
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
2021-12-06 17:36:53 +00:00
WMDE-Fisch 1467800331 Remove temporary hacks that got fixed upstream
Fixes are part of OOUI v0.42.1

Change-Id: Ic133d6cc503538fb8b4319bed037bc6e8596bd1c
2021-12-02 13:27:40 +01:00
Thiemo Kreuz 29ef00fa29 Rework all code using template type related conditions
This is a more radical change, compared to the previous patch.
I will post more detailled explanations as comments on Gerrit.

Change-Id: I6909b3f0b2c153b7ee9995441e995ffa793eab40
2021-10-22 10:42:16 +00:00
Adam Wight 5a451e0dd8 Workaround to prevent removing the selected item
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
2021-10-20 00:07:18 +02:00
Ed Sanders 295d139ef1 Minor documentation fixes
Change-Id: I6822c04c8d160431510e62752783e6829950dc36
2021-10-13 14:02:57 +01:00
Thiemo Kreuz be789e6387 Fix first click on parameter page causing bad scroll
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
2021-10-07 15:04:20 +02:00
Thiemo Kreuz 1904698105 Move some logic into the TransclusionModel class
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
2021-10-06 16:00:18 +00:00
WMDE-Fisch 6c7f303c86 Focus initial/single template placeholder
This includes some moving of code. These helper functions seem to make
more sense in the TemplateDialog class.

Bug: T292371
Change-Id: I004405bab60a569b084f9083fefa41f44f9a5561
2021-10-05 12:58:55 +02:00
Thiemo Kreuz 1731f3cc14 Better name for .selectPartByPageName() method
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
2021-10-01 07:00:40 +00:00
Thiemo Kreuz 7bd90b7506 Hack out forced scroll to first BookletLayout element
As a temporary standin for the mentioned OOUI patch.

Change-Id: I85d1fe542e10d83acdc0208d7cadc7a665eda0b2
2021-09-29 13:19:40 +02:00
Thiemo Kreuz 152f5edae2 Add/remove some TODO comments
Change-Id: I99bf367025ba4830fb683add93087d42dce4566d
2021-09-28 09:55:47 +02:00
Thiemo Kreuz 19bb1810c4 Rename misleading updateOutlineControlButtons event and handler
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
2021-09-26 11:50:38 +02:00
Thiemo Kreuz 63d9969861 Rename outline "container" widget
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
2021-09-24 12:32:29 +02:00
jenkins-bot 654cf8c3a8 Merge "Document the 3 ways the template dialog can be opened" 2021-09-24 09:45:56 +00:00
Adam Wight b94f7e02f5 Don't automatically focus the first parameter of each template
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
2021-09-24 08:42:05 +00:00
Adam Wight 41bb9e95e6 Don't automatically focus first element
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
2021-09-23 14:50:42 +00:00
Thiemo Kreuz 91d24bf2ec Document the 3 ways the template dialog can be opened
Change-Id: I9729eb238f4a4f0712a561b77e3d97f4e9063938
2021-09-23 16:38:33 +02:00
Thiemo Kreuz d6aece09cd Disable unwanted auto-scroll feature in template dialog
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
2021-09-22 13:30:37 +02:00
Thiemo Kreuz e037f99930 Merge custom "focusin" with "set" event handler
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
2021-09-21 10:14:57 +00:00
Thiemo Kreuz 9e6def01d3 Scroll template dialog element into view when pressing space
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
2021-09-20 09:08:44 +00:00
jenkins-bot 866052c7b2 Merge "Fix Cite dialog failing with new sidebar" 2021-09-20 08:23:32 +00:00
jenkins-bot 4f34e9b049 Merge "Regression: Update selection in new sidebar relative to content" 2021-09-17 16:53:29 +00:00
Thiemo Kreuz 48b9a58737 Regression: Update selection in new sidebar relative to content
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
2021-09-17 16:45:24 +02:00
Thiemo Kreuz d91e356780 Fix Cite dialog failing with new sidebar
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
2021-09-17 16:15:20 +02:00
Thiemo Kreuz 9f783d962a Regression: Fix selection of first transclusion element
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
2021-09-17 14:46:06 +02:00
Thiemo Kreuz cb685d1216 Fix enter sometimes focusing the wrong template part
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
2021-09-14 18:14:05 +02:00
jenkins-bot 401319cb67 Merge "Split focus handling from add/remove parameter events" 2021-09-13 13:28:16 +00:00
jenkins-bot 0cfd298d18 Merge "Fix and test page name splitting code in template dialog" 2021-09-13 12:23:17 +00:00
Thiemo Kreuz 6cb287c225 Fix space bar on top-level template parts loosing focus
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
2021-09-13 10:34:28 +02:00
Thiemo Kreuz 1493aa7a35 Split focus handling from add/remove parameter events
* 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
2021-09-11 17:47:02 +00:00
Thiemo Kreuz 22757fa901 Fix and test page name splitting code in template dialog
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
2021-09-11 17:40:39 +00:00
jenkins-bot 836ab09a79 Merge "Rename and document select/focus events in template dialog" 2021-09-03 09:38:21 +00:00
Thiemo Kreuz 06cf00f4c6 Rename and document select/focus events in template dialog
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
2021-09-01 16:23:59 +00:00
Thiemo Kreuz b0ddcb0010 Highlight corresponding template parameter in new sidebar
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
2021-09-01 11:15:37 +02:00
Thiemo Kreuz 99558f9076 Connect focus event from BookletLayout to new sidebar
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
2021-08-30 17:00:52 +02:00
Thiemo Kreuz 0ba7480901 More complete top-level part selection support in new sidebar
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
2021-08-27 19:16:00 +02:00
jenkins-bot 4741abe803 Merge "Added new input page for undocumented parameters" 2021-08-26 07:26:41 +00:00
Svantje Lilienthal 5c9fbca085 Added new input page for undocumented parameters
Bug: T272487
Change-Id: Ic5dcd36c9f647f9b52b98e5a520a1df1960a5b48
2021-08-24 10:18:43 +02:00
jenkins-bot f1c9012916 Merge "Remove dependency from new sidebar to the BookletLayout" 2021-08-23 14:40:02 +00:00
Thiemo Kreuz 3b5bc1b0b2 Fix non-existing property name .isNewSidebar
This is obviously a rebase mistake. We renamed the property in
I78a5129.

Change-Id: I43247ebf6c8910d12c90aa2dd1f9111f4ed69a07
2021-08-23 15:44:28 +02:00
jenkins-bot d6316145b5 Merge "Better method names in MWTransclusionModel class" 2021-08-20 11:29:49 +00:00
jenkins-bot 112bd2e5cb Merge "Call focusPart() only once in MWTemplateDialog.onReplacePart()" 2021-08-20 09:56:52 +00:00
jenkins-bot cdf7d907a9 Merge "Fix null error in ve.ui.MWTemplateDialog" 2021-08-20 09:48:27 +00:00
jenkins-bot 928b33ce69 Merge "Re-add adding undocumented paramters in some cases" 2021-08-20 09:33:18 +00:00
Thiemo Kreuz 8fbdd4258f Fix null error in ve.ui.MWTemplateDialog
Introduced via Ifd52369 just a few minutes ago.

Bug: T288827
Change-Id: I859f606c0608f9725bb64ff56a501a7a2c85c1df
2021-08-20 09:24:56 +00:00
WMDE-Fisch c3e5ea2fe1 Re-add adding undocumented paramters in some cases
Bug: T272487
Change-Id: I862c2802da1340d302a2a95e8e9d17bd6fd35877
2021-08-20 10:13:20 +02:00
Thiemo Kreuz 5fcc862eb1 Remove dependency from new sidebar to the BookletLayout
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
2021-08-20 10:07:03 +02:00
Thiemo Kreuz 7045371a4d Better method names in MWTransclusionModel class
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
2021-08-20 10:00:10 +02:00
Thiemo Kreuz 301c84bda4 Call focusPart() only once in MWTemplateDialog.onReplacePart()
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
2021-08-20 09:49:30 +02:00
Thiemo Kreuz 47bed4e1c8 Use page ID string instead of object reference
You will see where I'm going with this in the following patches.

Bug: T288827
Change-Id: Ifd52369d19c01499fb9cc801c3deec46c26810fd
2021-08-20 09:47:57 +02:00
jenkins-bot fdde92f820 Merge "Refactor onReplacePart in TemplateDialog" 2021-08-19 13:40:52 +00:00
jenkins-bot 0bf5c8493b Merge "Remove parameter adding on new sidebar code" 2021-08-19 13:40:16 +00:00
WMDE-Fisch eeda931f59 Refactor onReplacePart in TemplateDialog
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
2021-08-19 08:19:01 +00:00
WMDE-Fisch d41714f3a1 Remove parameter adding on new sidebar code
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
2021-08-19 08:18:55 +00:00
Thiemo Kreuz 0af0978db4 Use same .useNewSidebar property name everywhere
This makes it easier to find stuff.

Change-Id: I78a51290767db4fe06ba0d4d2d251e8df07368b4
2021-08-19 10:13:16 +02:00
jenkins-bot eb0e2cd507 Merge "Change filterParameter(s) event to work with a list" 2021-08-16 11:10:09 +00:00
jenkins-bot 41c286c68c Merge "Rename ambiguous "add button" click handlers" 2021-08-16 11:01:36 +00:00
Thiemo Kreuz 0bbcb436c1 Change filterParameter(s) event to work with a list
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
2021-08-11 08:06:22 +02:00
jenkins-bot 004d37dc1d Merge "Hide old sidebar when the new one is enabled" 2021-08-06 13:10:04 +00:00
Thiemo Kreuz 3e5a2d6c82 Filter right content side of the template dialog as well
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
2021-08-05 14:36:10 +02:00
Thiemo Kreuz f37814ba61 Rename ambiguous "add button" click handlers
Change-Id: I25fd1430443feffcc3fb17115dc4129890063c48
2021-08-05 10:51:26 +02:00
Thiemo Kreuz 0351537ba2 Hide old sidebar when the new one is enabled
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
2021-08-05 09:14:40 +02:00
jenkins-bot e57daaad98 Merge "Minor JSDoc documentation fixes in template dialog code" 2021-07-29 12:13:17 +00:00
Thiemo Kreuz 5c6d2212da Make it possible to reuse existing transclusion outlines
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
2021-07-16 14:43:31 +02:00
Thiemo Kreuz 092276009a Minor JSDoc documentation fixes in template dialog code
Change-Id: I3697e48697ab0910b90b418abbc370736589b6e1
2021-07-16 14:25:11 +02:00
Thiemo Kreuz 434c11f6de Fix incomplete template dialog event handling in new sidebar
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
2021-07-16 11:29:26 +00:00
Thiemo Kreuz 8ab2ec25ea Fix template editor showing more than one new sidebar
Bug: T274544
Bug: T286715
Change-Id: I9fe8b53a73b1862eca2d99afb02aa3d532d61918
2021-07-15 15:17:44 +00:00
Thiemo Kreuz ff9f8a9e2d Focus template editor elements on click in the new sidebar
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
2021-07-15 08:40:28 +00:00
Thiemo Kreuz 226e2657ef More robust debug container for new template editor sitebar
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
2021-07-13 09:11:57 +02:00
Thiemo Kreuz 1c98f4cce0 Remove unnecessary code from template related classes
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
2021-07-07 10:18:41 +02:00
jenkins-bot 9e6387176b Merge "Add @private/@protected documentation to template dialog code" 2021-07-06 08:41:55 +00:00
jenkins-bot 7afb7981d1 Merge "Rewrite MWTemplateDialog.setApplicableStatus for readability" 2021-07-01 11:56:30 +00:00
jenkins-bot 5028f4eaf3 Merge "Remove unused "insert" action from MWTemplateDialog" 2021-07-01 11:52:21 +00:00
Thiemo Kreuz f1a66c17e4 Rewrite MWTemplateDialog.setApplicableStatus for readability
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
2021-07-01 11:20:50 +02:00
Thiemo Kreuz 55a49195ba Add @private/@protected documentation to template dialog code
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
2021-07-01 09:36:00 +02:00
jenkins-bot 7426139a41 Merge "Fix possibly uninitialized variable in template dialog" 2021-06-29 13:47:06 +00:00
Thiemo Kreuz (WMDE) e6d3828429 Revert "Extract "show all" to placeholder class"
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
2021-06-28 14:04:12 +02:00
Thiemo Kreuz 6b346442ad Hotfix for broken "Extract show all to placeholder class"
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
2021-06-27 18:29:23 +02:00
Thiemo Kreuz 2966b69609 Fix possibly uninitialized variable in template dialog
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
2021-06-25 16:49:30 +02:00
jenkins-bot 118ae98a2a Merge "Wire model changes into new template sidebar" 2021-06-24 09:31:46 +00:00
Adam Wight 64c5b093cc Wire model changes into new template sidebar
Only partially wired.

Bug: T274545
Change-Id: I961ffbdd82829b18c08a7c33759a05427103fa8b
2021-06-24 11:09:39 +02:00
Thiemo Kreuz 4367235dcc Inline many var declarations in the code below
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
2021-06-23 09:02:24 +00:00
WMDE-Fisch 407ff95597 Rename OutlineParameterCheckboxWidget to OutlineParameterCheckboxLayout
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
2021-06-21 09:08:54 +02:00
Thiemo Kreuz c135f10f72 Remove unused "insert" action from MWTemplateDialog
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
2021-06-18 13:44:35 +02:00
Thiemo Kreuz 2a1defc26f Remove unused .onAddParameterBeforeLoad() method
https://codesearch.wmcloud.org/search/?q=onAddParameterBeforeLoad&i=1

Bug: T284895
Change-Id: Ib521e05d8def8514ffe1b3c3822f0151ab10932e
2021-06-17 18:30:02 +02:00
Adam Wight 988cda65dd Template dialog checkbox list
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
2021-06-17 08:47:47 +00:00
jenkins-bot 6d101fb9d7 Merge "Extract "show all" to placeholder class" 2021-06-16 12:25:55 +00:00
jenkins-bot 4c7f5ff805 Merge "Introduce a new class to encapsulate booklet inter-pane wiring" 2021-06-15 14:55:16 +00:00
Adam Wight 950a5300dc Extract "show all" to placeholder class
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
2021-06-14 18:09:26 +02:00
Thiemo Kreuz c2017f74b2 Remove @param/@return docs that literally repeat the code
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
2021-06-14 15:44:51 +02:00
Adam Wight 44035a2384 Introduce a new class to encapsulate booklet inter-pane wiring
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
2021-06-10 15:30:51 +02:00
Thiemo Kreuz 7fdc560fb8 Rename "sequence" to "ordered parameter names"
There are 2 methods with the same name, but they are very
different. This makes it much easier to understand the
difference, I hope.

Change-Id: Ie1f049b2b14e1fe23f078e281ee797da29dfe3db
2021-06-07 23:24:17 +00:00
jenkins-bot 69676950b3 Merge "Much longer descriptions of template dialog related classes" 2021-06-04 12:00:46 +00:00
Thiemo Kreuz e13b0dae48 Much longer descriptions of template dialog related classes
The idea is to possibly rename some of these classes, based on
these descriptions. But this should be done in later, separate
patches.

Change-Id: I7f9e5b2382711b434d6dd618489fa3ed8b7a46b4
2021-06-04 13:17:59 +02:00
Adam Wight d4eee82701 Update template dialog titles: Insert/Edit
Toggle depending on whether the transclusion existed or is new.

Bug: T276568
Change-Id: Ib9b76cac7cd57245e8db2ef10879069a86a6269e
2021-06-02 12:36:38 +02:00
jenkins-bot fa94c27a8d Merge "Change header titles in the template dialog" 2021-06-02 09:05:04 +00:00
WMDE-Fisch 58cf1b0344 Change header titles in the template dialog
Bug: T276568
Change-Id: I5da40032c5b83d04ae92e86372c445d7a28d0444
2021-06-02 08:48:07 +00:00
Andrew Kostka c1d36072ea Add collapsible descriptions to the transclusion dialog
Bug: T273426
Change-Id: I793a66f5c5877451b70058e9388dd86b3fcaadd7
2021-05-10 11:18:17 +00:00