Commit graph

163 commits

Author SHA1 Message Date
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