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
This is a direct follow-up to I6ebd020.
Steps to reproduce the bug:
* Make sure you have a template with a deprecated parameter.
The position doesn't matter.
* Add the template. The deprecated parameter is hidden.
* Add an undocumented parameter, e.g. "b". This is added to the
end, as it should.
* Add an undocumented parameter "a". This should appear before
"b", but doesn't. The reason is because the invisible
deprecated parameter is in the list that is used to calculate
the index, shifting it by 1 (or more when there are more
hidden parameters).
This patch includes a few closely related changes:
* We can loop the list of checkboxes directly instead of
indirectly via the list of parameter names.
* I made it so that an active filter only resets if it would
hide the new parameter. The original problem we had to solve
was that the new parameter would always be visible, even if
it doesn't match the filter. This awkward mismatch is still
guaranteed to not happen.
Bug: T274551
Change-Id: I1b0480ae836cc19b77b159d3fb30ff32e8c59df4
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
This affects only the new sidebar. Deprecated parameters don't
get a checkbox, except they are used already. "Used" includes
parameters that are present, but empty.
Bug: T274551
Change-Id: I6ebd020d02650c19060345d13495373acab363df
Disambiguation pages are rarely the page users intend to link to,
especially with newbies. By moving the disambig page(s) as the last
result, the user is more likely to pick the page they actually intended
to link to.
Bug: T285510
Depends-On: I2b8545f6dd4849629037f81f48a540748e60da83
Change-Id: Id55a19e7665d8f88559c471de36e5447fb2babb0
With this users can also filter undocumented parameters
that they added to an empty starting template.
Bug: T272481
Change-Id: I99adb38b0ae4d4ade91fcb506f10c0222b9bb5e8
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
This ellipsis was there before we started working on this code,
but was never working properly.
We understand that the CSS was intentionally done like this (as
the comment explains). However:
* We changed the width of the dialog. The old value doesn't
match any more.
* The width is different when the sidebar is expanded vs. when
it is collapsed. Even if we update the number, it won't
always work.
* The 100% work fine in current browsers. I can only assume
this was different back in 2014 when this CSS was written
(see Ia8259e9).
Bug: T285044
Change-Id: I3de2b0ed0b6a05d2b9fa0b325a2b12277564b271
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
Notably:
* Include parameter aliases, labels and descriptions in the
search.
* Don't use a possibly outdated search index, but live data.
* Clear filter when a new checkbox is added.
Bug: T272481
Change-Id: Ie90a803af6178a8bb6de370a0f8e079800d9f8a2
In detail:
* Allow clicks on all elements in the new sidebar. This should
focus the corresponding element on the right.
* Make all elements in the new sidebar tabbable.
* Fix MWTransclusionOutlineTemplateWidget.createCheckbox() to
not need a temporary param object any more.
* Rewrite more code in MWTransclusionOutlineTemplateWidget to
be shorter and easier to read.
* Fix MWTemplateModel.addParameter() to not do way to much
stuff when a parameter already exists.
* Update code documentation.
* Use more specific, less ambiguous variable and method names.
Bug: T274544
Change-Id: Iaf6f7d1b0f7bf0e9b03eb86d01f3eceadece6fe4
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
Actually reusing this OOUI mixin gives us a lot of well
developed functionality we need anyway. Most notably proper
event management, e.g. click events.
The number of CSS properties we need to override is managable,
I would argue. Let's see:
* Our buttons are not inline-elements, but should use the full
width.
* No focus-border left and right for the same reason.
* We want much more inner padding.
* We want a stronger hover effect.
* We need to fine-tune the position of the icon. This is
because of the inner padding.
* Need to get rid of a negative margin that's only relevant
for inline-buttons.
I currently feel like the benefits are worth living with
slightly more brittle code. Note that we can undo this change
any time because all this is well encapsulated in this new
class.
Bug: T274544
Change-Id: I33f275a958964d49e803e56bf74a6fa961093da1
This introduces another generic "button-like" class that can
be reused in multiple places in the new sidebar. The main
change in this patch is the "add more information" button
which is now an instance of this new class as well.
This patch also simplifies over-complicated setup code in
related widgets.
Bug: T274544
Change-Id: I0cfe7675d02fdd5c5dc8d9198bb3f4aec9abf397
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
The two new widgets are pretty trivial now, thanks to the base
class.
Note there is still no code to delete the widgets. That's also
why you will always see a placeholder widget at the top. This
will be fixed with the next patches.
This patch also renames most of the "…TemplateOutline…" classes
to "…TransclusionOutline…" The reason is that these widgets are
not for a single template, but part of the container widget for
a more complex transclusion (i.e. a sequence of multiple
templates and wikitext snippets).
Bug: T274544
Change-Id: If4219b0b8ad4d1969ab1ec5ec4db0728811bab35
The icon and the name of the template are now created by the
base class. This is meant to be reused for other elements
that are not templates.
Bug: T274544
Change-Id: I76bbc0e8c0420e9c6357d093d5f5e1651a0c2719
Reasoning:
* format=json must be the default. Nothing else makes sense in
the context of this code. This should not be a surprise.
* formatversion=2 is only a default when the custom
getContentApi() is used, but not when mw.Api is used. One
might argue that it's safer to always specify formatversion=2.
However, this is not done in other places in this codebase.
It should never be done or always.
* I find it confusing when the action=… is missing. Let's not
rely on this default.
Change-Id: I6ca29f76bffc0849103c5bcff4aaf28fcaaa4c52
This code doesn't do anything but adding an empty <div> to
something that is already a <div>. It doesn't even have a
class name, i.e. it's not referenced from anywhere. We can
add such containers back any time when it turns out we
actually need them.
Bug: T274544
Change-Id: I62546cc7939364db31f37b9de0c035974554544b
This base class will be used to style the 3 types of top-level
items in the sidebar the same way, without the need to
duplicate code or styles.
Bug: T274544
Change-Id: I1a62ff610728d7150dea1717316ef20f6882783a
This matches the existing naming scheme better. I also plan to
re-use this class for other types that are not templates.
That's why the name is the more generic "transclusion" now.
This patch also removes a `padding: 2px` that's not that
helpful. We will need paddings later, but need to choose them
much more carfully.
Bug: T274544
Change-Id: I6f0f630da2230b023b3fb065e5ad86d8211bb7b3
Because the API uses a generator, the search results are ordered
alphabetically. The actual search result ranking is in a .index
field. This code accidentially deleted the alphabetically lowest
template instead of the least relevant one.
Change-Id: I79de024feb569e9f06bedab908a6509a4d4fa99b
We do this additional prefixsearch anyway. What we did before
was ignoring the result when it was not a 100% exact match.
Instead we can always add this 1 prefixsearch result when it
was not already part of the CirrusSearch result set.
This won't happen often. Usually the 1st prefixsearch result
was already part of the CirrusSearch result set anyway. But
if it wasn't, that's a serious issue for expert users that
expect the search to behave similar to the suggester at the
top of the MediaWiki interface (which is also a prefixsearch).
Change-Id: I959d2b058a3d64596a8cfbe5476ab351e40f8760
Some details:
* The config is not optional in these cases.
* This patch continues to remove some comments that don't add
any information but just repeat what the code already says.
Change-Id: I5c27cd01ad80709bb583256821d65c6b65b74b05
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
It's good practice to make transparent elements transparent
for mouse clicks as well, i.e. make it possible to select text
behind the fade effect.
Bug: T283943
Bug: T286235
Change-Id: Ib5022a74c70e4b7cb5e2a0faad20bd9abcc0da36
Introduced in 2 separate patches by the same author. This
patch removes the line that was introduced last.
Change-Id: I77575f7afe0f9276c7b54ee44d828e7ccb87c978
These methods are special in so far that they create *minimal*
wikitext where optional whitespace is not preserved. I tried
to rename the methods to reflect this, but could not find a
caller. What's used instead are the .serialize() methods.
Bug: T284895
Change-Id: Iedaa5b7efa9675151cc0553854d8aef3f9a46cbb
If a known parameter is present using one of it's aliases, then
only the aliased name should be shown to the user. This patch,
therefore, resolves the issue of the same parameter being added
to the sidebar twice.
When adding a parameter that is aliased, it will receive the same
position as the non-aliased parameter it is replacing.
Bug: T274545
Change-Id: If4e58c941fd0f0e690d3603935f5a5d3f9938163
It's allowed in values, but not in parameter names. The moment
a parameter name contains an `=` the parameter name will be cut
off at this point, and what's behind the `=` will become part
of the value.
You can test this on any live wiki. Open VisualEditor. Edit any
template. Add a parameter with a name like `a=` and some value.
Switch to wikitext mode and back. Edit the template. The `=` is
now part of the value.
Bug: T98065
Change-Id: I5e00e8fac987471243605816b041d3638927ac3b
When what you type is a partial match, you can't add it as an
unknown parameter, even if that would be the correct action. The
reason for this unexpected edge-case is a mistake in the code
where a variable called "exactMatch" is set when a *partial*
"nameMatch" was found.
Bug: T285940
Change-Id: I6d12e2d7251a19d7d5f8be544c3c32a3ac14fcf0
The so called "spec" class keeps track of parameters that have
been used before, no matter if documented via TemplateData or
not. Removed parameters are still "known" (i.e. have been seen
before).
This feature allows to easily find previously used parameters
names when an undocumented parameter was removed and the user
tries to add it again.
Bug: T285483
Change-Id: Ia1555eea87cd99e7a3f386f4279ec5a80fb98a79
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
While the term "canonical" is not wrong, I find it still
somewhat ambiguous.
1. "Canonical" could mean different things. E.g. is the order
of parameters as they appear in the article's wikitext the
"canonical" one? It's possible to argue like this, esp. if a
template doesn't have TemplateData documentation. In this case
the only order known is the one from the wikitext.
2. "Canonical" sounds like the parameters must be reordered.
But this should never happen. Not having dirty diffs is more
important than having the parameters in a specific order.
Bug: T285483
Change-Id: I23658d37fea50b727667677ac6a49066673b2135
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 reflects much better how this method is meant to behave.
Note I will continue to remove documentation that doesn't
explain anything in addition to what the code already says.
Bug: T285483
Change-Id: I81fa8a5d9d0752f3aeac4015c9a27b50e054d4df
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 will avoid that the search breaks in edge cases where symbols
are used.
Including a fallback for ES5 browsers. The fallback should cover
almost all cases. Worst case would be not adding the asterisk even
though it might be valid.
Bug: T284554
Change-Id: Ie4aee0b77492b7a73bc251a8723a206dbd641600
We have to move the activation logic from a ve.ui.Command to the
ve.ui.Tool, as the Command is unable to refer to the Target.
Previous attempt: 4984c5ffbb
reverted in fb32aa4978.
Bug: T279313
Change-Id: I3cb74aa5123b67a6c63b8e07ea7f93a6d4a07d4f
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
Both the template description as well as the parameter
description (including default value and examples) typically
contain longer texts. These can contain longer words that
"explode" the design. This is trivial to avoid.
Note this is not meant to fix this issue in all places where
it can appear. For example, a long parameter name causes the
same issue. But:
* Technically, it's not that easy to fix.
* Even if, it's not obvious how to fix it. Cut off the
container? Add ellipsis? Or wrap? How should the
surounding stuff float then?
This is all left out because of this. Focus on what's
obvious.
Bug: T284890
Change-Id: Id6700af168f5ab5ddde97d3f5ae63829b65a3be5
* Re-focus the input field after closing the message.
* Store only the message key. That's all that's needed.
* Avoid a class property that's not needed.
* Use the config object instead of calling .setLabel() manually.
Bug: T284742
Change-Id: If8e8bb6460fa5aea8ddd46c2e27b5f08b7772896
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
This dramatically simplifies the "mode" flag in
MWTransclusionDialog. The main reason to touch this code is:
The flag appears like it will be "single" when the dialog
contains a single template, and "multiple" when there are
multiple templates. But this is not true.
What the flag really does is show/hide the sidebar. The sidebar
is needed to be able to create multi-part templates. But a
dialog that already contains multiple templates can be set to
"single" mode (i.e. the user can collapse the sidebar), and
vice versa.
This patch focuses on private details inside of this class, but
keeps the terminology of a "mode" in some places. E.g. the
messages are not renamed to not cause unnecessary trouble for
translators.
Change-Id: Ib029fd48b393d2ab7d7cff6c842789e22989e944
Previously, if the checkboxes were shown on multiple lines (e.g. due
to a FlaggedRevs checkbox), there would be uneven margin at the
bottom. There was a special case to fix this only for the watchlist
expiry not-checkbox.
Change-Id: I006049cf23e6d42519bfa15b7ec30ea1bc5d08ac
This class represents a raw wikitext snippet. There is also no
base class that would require us to follow a generic
getValue/setValue naming scheme.
Change-Id: I0891a2f6c0ae0121429a47c39221e99b9653e8e3
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
This does have a few advantages:
* Less code is executed and less memory consumed when these
elements are not needed.
* Code that belongs together is together.
* No local class properties are created when they are not
needed in the code below.
This patch is kind of a proof of concept. It touches only a few
classes we currently actively work with. If this change is fine
we can change some of the other classes the same way.
Change-Id: I9f548765034f1f69799fff41aeb6c147ff28b82d
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 main motivation for this patch is actually the comment. The
so called "spec" contains all parameters that are present in a
template, no matter if they are present in the TemplateData
documentation or not. This is critical here.
Change-Id: I5e1c79e3859a27562a9dea1d450cec196aa572ed
Depends on whether this is a new or existing template transclusion.
Split from Ib9b76cac7cd57245e8db2ef10879069a86a6269e
Bug: T276568
Change-Id: I4d22e32fef067b640e9a9389deffaace736c3405
When the existing search results don't contain an exact match
(see previous patch), perform an additional search for the
title. This uses OpenSearch. This is recommended in multiple
places and also used in the quick search field at the top of
MediaWiki.
Again, I came to the conclusion that an isolated unit test
would be complicated and not test much anyway. Better test
on-wiki.
Bug: T274903
Change-Id: Ib575248e089ff66814400202d224deff6369c772
This code detects a few edge-cases:
1. When some search results are exact matches, make sure they
are always at the very top.
2. When the prefixsearch API is used, e.g. as a fallback,
redirects show up as a separate metadata structure outside of
the pages array. Consider these and stop if there is already
an exact match.
3. CirrusSearch returns redirects as part of the pages array.
When there is an exact match, make these redirects separate
options and add them to the top.
All of this is case-insensitive, on purpose. In case two
templates with different capitalization exist, we rely on
the backend to return both. The code introduced here is fine
with this.
Notes:
* This doesn't guarantee an exact match is always there. This
requires an additional HTTP request and is done in the next
patch.
* I tried to write unit tests for this, but gave up. The setup
is complicated. An isolated unit test would not test much
anyway. Better test this on-wiki.
Bug: T274903
Change-Id: I64e1b5633e7b878a4d0d23d66229ca87e69d0045
These are the most minimal (and therefor most stable,
hopefully) hacks I could come up with so far.
Bug: T274903
Change-Id: I28ba414dd34aad756e29400eb656f0942291a923
* Create getSurfaceClasses method.
* Pass surfaceClasses to target widgets.
This ensures that the 'content' class is passed to mobile
target widgets, and the 'mw-body-content' class is added
in a less hacky way.
Change-Id: Ibce6d1a1d0fda63cca354761f1b91f808858e95b
Template names sometimes show up twice when searching for a
template in the "Add a template" dialog.
This is a bit hard to test. The code responsible for this
is not in a single place. The feature is in the upstream
TitleWidget class. It's not broken. It makes sense to
provide e.g. "foo" and "Foo" as two separate options when
the user typed "foo", but the page is named "Foo". Both are
valid, and the feature allows the user to pick either.
But the VE widget does it's own normalization. Both entries
are normalized to "Foo". Both do the same. The additional
one is pointless.
You can try this on the actual enwiki: Open VE, insert a
template, search for "Template:nHLE".
Change-Id: I65e706c4d131a2f8c605d7979a02ea56f831bf03
The "redirects" part in a prefixsearch query is always an
array, no matter if formatversion 1 or 2 is used.
The "pages" part is an object with formatversion 1, and an
array with formatversion 2.
As of now this always uses formatversion 1. This is
hard-coded in the upstream TitleWidget class.
Change-Id: I8cde8e104f8a288015da745db41016f6639b453b
Returns true if there is no meaningful user input yet.
Will be used in the next patch.
Bug: T272355
Change-Id: I4f88ce31662bbc46755f78d574c46b907581d438
Rather than invent our own size, we'll reuse the "larger" format and
tweak the dialog height to 90%.
Bug: T273971
Change-Id: Ibef85c1912267b14d83396b089b81934751a8328
Discussed in T274903#7077957. Note this might not be the
"perfect" solution. We are still experimenting, and this is
all hidden behind a feature flag. This is the change with the
most minimal impact. Actively trimming the input is another
solution, but with a bigger impact we might want to discuss
first.
Bug: T274903
Change-Id: I2ed06c04bb96c7b61bd7e87ad001e639ea6d06a2
We have two cases now that we want to cover here:
- Either we're inserting a new template and start a "fresh"
transclusion, then we want to use "search" in the headlines
- Or we're adding a new template to an exsisting
transclusion, then we want to use "add" in the headlines
Bug: T277028
Change-Id: I9fa294cf732598d58f848c75b353d2e1742eb4e8
This allows using the config variable independendly from the cirrus search extension.
This way it can be used for all subtickets of T271802.
Bug: T277028
Change-Id: I1b3bdda5fa6fbfe5c531c3b51c2c8e2a28ed1faf
Renames "Add a template" to "Template Search" in most cases and
provides inline help for the workflow.
Bug: T277028
Change-Id: I3fee87cb89b5044e785596e71ef3f1a18f2694ce
We're about to replace this jQuery element by a OOUI container, and
can take an initial step by reducing its lexical scope.
Change-Id: I4123c8d22c01040fc2f61180304254498b21f5fd
The name "description" conflicts with the TemplateData field name,
which is only one of several documentation fields.
Change-Id: I0942701204fe8499e8890740585b9a02c1d14c63
The internal name "more" conflicts with new collapsible buttons.
TODO: looks like TemplatePage has an analogous field?
Change-Id: I10b24758316a6cc3fbd236c77daffa014fcdafc6
When $wgVisualEditorTransclusionDialogInlineDescriptions is set to
true, the template dialog will use a larger format.
Bug: T273971
Change-Id: Iad3c3f4d65125c83e35414ce15f793f6a1b192ef
This would would be correct if it was in a method of ve.ui.MWBackTool,
but this is actually a different class ve.ui.MWBackCommand (they are
both in the same file), which doesn't have a 'toolbar' property.
This partially reverts commit 4984c5ffbb
"Avoid using mw.ArticleTarget methods on global ve.init.target in tools".
Bug: T279613
Change-Id: Ia5e80e34cc5021295639cf18b3c324d2821aecf5
Triggered by Ctrl+Shift+Space on PC.
On Mac, there is no trigger, we rely on the built-in OS shortcut.
Bug: T53045
Change-Id: I9630804c833d755910589022b0ca78208337d804
As far as I can tell, the code only uses the comment, nothing else.
Omitting the title probably won’t make the underlying database query any
cheaper, but it should at least save some network traffic.
Change-Id: Ideb66ce3a24fb4f42fe8fc22ba0e93d05724d8b6
Previous, reverted attempt: da9b6fffbd.
This attempt also includes 6037fefbe0,
and fixes minor conflicts with other changes.
* In normal images, parse relative 'href' attributes instead of
expanding them to absolute. This resolves Parsoid generating
|link= options for copy-pasted images (T193253).
Keep them in the underscore-form to avoid causing dirty diffs like
T237040 again. Unlike in the previous attempt, we don't need to be
super-careful about the 'resource' attribute, thanks to the Parsoid
changes in T108504.
* In gallery images stuff, prefix the 'resource' attribute with './',
same as normal images do. This causes no functional changes, but it
makes updating tests easier, and the consistency is probably good.
* Update test examples to also prefix 'resource' and relative 'href'
attributes with './', like the real Parsoid does.
Bug: T193253
Change-Id: I91131728a87c9406bf069d46d3c94c9a8905a003
The conversion to a DM doc and subsequent cleanup is a generally
useful step that should be available as its own method.
Change-Id: Ia53c0a641b231bb81c25c011624357acf4dc42a3
This was used when we used to pass API errors to showMessage, but
is now unused by the two remaining users (missing edit summary, and
"press ctrl+enter to submit").
Change-Id: I8a6b4db78d4e451cf3ec85fcdfd8293328aaaa3c
