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 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
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
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
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
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
The names in the schema are roughly following what's
done in Schema:TemplateWizard. The information if
templates have TemplateData will be logged seperatly.
Bug: T259705
Change-Id: Iafa7256f675dbfd6a5a6de794061901780e3c55d
New changes:
202adf904 [BREAKING CHANGE] Unify FragmentInspector/Dialog behaviour
Local changes:
* Update dialogs to use common actions & FragmentWindow
Change-Id: Ib744b8996db48d1ee58bc873120400566c490e88
I have moved this block of code to the wrong place in change
13675e4a81. As a result,
`this.loaded` was being set early, so the dialog was treating
all of the pre-existing transclusion parts as newly inserted,
and the "Apply" button was therefore always enabled.
Bug: T209661
Change-Id: I3c1b45f91738ab6fc4a6f6d61ae5bf925c9a1bb5
I ran Closure Compiler over the codebase just to see what would happen,
and it printed some useful warnings.
Change-Id: I56d40b11e6d1dd7ce68a5e59da511f66e928647f
This is similar to the hack in ve.ui.MWMetaDialog, except uglier :(
We already explicitly focus the right field in the ready process.
I am not really sure why the focus change causes the issue, but
preventing it definitely fixes it. It would make sense if we changed
the value of the field after focussing it (as setValue() restores the
validity flag cleared by onFocus()), but we don't seem to do it.
Bug: T199838
Change-Id: Ia602551ee0b0885cefbd4cb2fc00d569ff42da67
The #getReadyProcess method should be used pretty much only to focus
a field inside the dialog after it is opened. It runs after the window
opening animation finishes, so if you add stuff to the window here,
that will be visibly delayed.
The #getSetupProcess method should be used pretty much for everything
else that depends on the opening `data`. It runs before the window
opening animation, so if you add stuff to the window here, it will be
visible while animating it.
Bug: T185944
Change-Id: I71ea5b6e1e1947c1cf8fd749100e854953a8ef3c
