Commit graph

54 commits

Author SHA1 Message Date
Ed Sanders bc0e96f78a tests: Use ve.test.utils.newMwEnvironment()
This calls "parent" methods automatically, and allow us to
pass a config parameter for MW config.

Change-Id: I571d4599d7fca55727070bcac6083d1232b1f61c
2022-03-10 01:44:11 +00:00
Thiemo Kreuz 836d36bc16 Skip initial creation of unused parameter widget
In a test case with 200 templates where all but a few parameters are
unused the loading time is cut in half.

Bug: T300974
Change-Id: Ice850cb9e5e95b9e3a19ff511b3a4f32117c7199
2022-03-02 10:19:46 +00:00
jenkins-bot 2a72f4e290 Merge "Remove not needed self = this indirections" 2022-02-21 12:57:41 +00:00
Thiemo Kreuz 3c042049bf Use .bind() syntax where it makes code more readable
This focuses on a few trivial cases where the syntax helps making
the code more readable. One level of indirection is gone with this.

Change-Id: Ibf25d7eaa06952e69b36bd5a78a48d04ac62890c
2022-02-21 10:34:03 +00:00
Thiemo Kreuz 58e27f56d6 Remove not needed self = this indirections
These are only needed when we need to access a specific `this` from
within another `function () {}` context. This is not the case in the
situations here.

This is split from Ibf25d7e to make it smaller and easier to argue
about.

Change-Id: Ide1476de91fc343aa992ad92a1321d3a38b06dd0
2022-02-21 10:52:24 +01:00
Thiemo Kreuz be5a752c8e Minor code cleanup in MWTransclusionOutlineTemplateWidget
Change-Id: I98904756e8ab9069634f644dc4d8631d1c0ee8fb
2022-01-23 17:09:46 +00:00
WMDE-Fisch 6cb0e7477e Hide parameter search when there are only 3 params
The search field is of not much use when there is not really anything
to search. It wastes more space than the actual list of parameters.

Approved by UX, see T298259#7626538.

Bug: T298259
Change-Id: I01784a1c463d8b0b504897b20179719f91597d19
2022-01-21 11:23:49 +01:00
Thiemo Kreuz 66f56f6c88 Defer creating template parameter search when it's not needed
From the user's perspective this is the same as before: When a
template doesn't have any parameters, there is no search field. The
moment the first (undocumented) parameter is added the search field
appears.

This is just delayed now. The widgets are only created the moment
they are actually needed.

This saves loading time and memory, especially in a multi-part
transclusion with many zero-parameter templates.

This also makes it a lot easier to change the minimal number of
parameters from 1 to e.g. 4.

Includes reverting the flexible header composition done in
Ib050e30a50ef965c1524e977d3a600c3ff836774

Bug: T298259
Change-Id: Ied7541d8d5c0b478a439dd31ce072e634287f181
2022-01-19 11:23:34 +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
Ed Sanders 33840e88ee build: Update eslint-config-wikimedia to 0.21.0
Change-Id: I19465a5ab3bf71cd97967fd1fac41c645f05a419
2021-11-10 14:52:56 -05:00
Ed Sanders 36eacb1095 assert.ok -> assert.true
Change-Id: I9987ae0cbe275bf492dcdef405454537119765ea
2021-11-09 00:30:20 +00:00
WMDE-Fisch dab5b5daae Add ARIA descriptions to sidebar widgets
When changing the source in the described-by attributes the screen
reader will read the text of the new source when the status changes.

Just changing the text within the elements holding the descriptions
does not work.

Bug: T291284
Change-Id: I31cc3061cf6c1f699babe41e99e0711f0eb03646
2021-10-29 12:07:53 +02:00
jenkins-bot c9222cf111 Merge "Fix strange dialog title when using subst:" 2021-10-25 09:12:13 +00:00
Ed Sanders e1fb20d8be Replace IIFE with block scope in ES6 code
Change-Id: I635f72a62ae44943a7705d28e92c9a0d265800b6
2021-10-14 15:11:45 +01:00
Thiemo Kreuz 74e3dd3f4b Fix strange dialog title when using subst:
When I type e.g. "subst:example" as the template name, we made this
work as the user would expect: the template named "Example" is found
and it's TemplateData documentation used. But the dialog title shows
"Subst:example". Note the uppercase "S". It means this string is
parsed as a title, including the "subst:". This is confusing. Just
show the template name.

Change-Id: I9817786991a8379cf48b0a664aef1413abddee2d
2021-10-08 16:00:29 +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 66d5f13dfb Remove obsolete …OutlineWikitextWidget test
The feature set is (almost) fully covered by other tests, notably
the tests for the …OutlinePartWidget base class and the other two
subclasses.

The only bit that's not covered any more is the
"visualeditor-dialog-transclusion-wikitext" message. But that's
super minor and not worth a separate QUnit test.

Bug: T291157
Change-Id: I574f9cff0baf3dff885094769c124a9e05a1d1c8
2021-09-28 08:37:47 +00: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
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
jenkins-bot fe6c46bbdc Merge "Merge "templateParameterClick" and "choose" events" 2021-09-15 10:11:12 +00:00
jenkins-bot 1666db730b Merge "Avoid expensive/unnecessary .getItems() call in a test" 2021-09-13 13:41:58 +00:00
jenkins-bot 26b3f5abf0 Merge "Consistent use of .forEach() in QUnit tests" 2021-09-13 13:41:28 +00:00
jenkins-bot ad27a6d7bc Merge "More assertions for ve.ui.MWTransclusionOutlineParameterWidget" 2021-09-13 12:24:33 +00:00
Thiemo Kreuz 338a800837 Merge "templateParameterClick" and "choose" events
We don't need to distinguish between these any more. Both are
"active", i.e. both focus the widget on the right side of the
dialog. Sometimes the "choose" event is fired to actually add
or remove a parameter. Sometimes it's fired, but the state of
the parameter doesn't change (for whatever reason, i.e.
because the parameter name was clicked instead of the
checkbox). There is nothing to do in this case, except for the
focus change.

Change-Id: I3c7c0c81a075ccff76eda0a4fb2aa1ac7be3cec5
2021-09-11 17:47:08 +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
Thiemo Kreuz f14eba17ee More assertions for ve.ui.MWTransclusionOutlineParameterWidget
These additional assertions cover previously uncovered features.

Change-Id: I6708ce2ef965f9ad81ec3d0aee163b6f2093344c
2021-09-11 14:17:35 +02:00
Thiemo Kreuz 5f37d4d140 Avoid expensive/unnecessary .getItems() call in a test
Calling .getItems() creates a copy. Using .items is possible,
but strictly speaking a private implementation detail.

Change-Id: Id9438faff88cb5ff3973bca4c959e74d9be7e823
2021-09-09 11:32:19 +02:00
Thiemo Kreuz f8cdcfe129 Consistent use of .forEach() in QUnit tests
* The extra brackets are not needed in ES6.
* Make sure the name of the test is on the next line. This makes
  the code easier to read.

Change-Id: Ib871dbfa27fcadc88e7335b9efb4d583bd4300ac
2021-09-07 14:27:25 +02: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 b7efea5559 Remove unused event listener from …ParameterSelectWidget
We forgot to remove this in I319896a. The individual
…TransclusionOutlineParameterWidgets don't fire this event any
more. Instead this is done by the …SelectWidget.

Bug: T285323
Change-Id: I2c29e45127464785ffdc32d73b52188fcbefb7bf
2021-09-01 11:58:36 +00:00
jenkins-bot 49eaab5139 Merge "Add tests for ve.ui.MWTransclusionOutlineParameterSelectWidget" 2021-08-31 12:04:02 +00:00
Thiemo Kreuz 5ffaa3e313 Add tests for ve.ui.MWTransclusionOutlineParameterSelectWidget
Note this covers both the outer SelectWidget as well as most of
the functionality of the item class. This is because the outer
widget manages everything. The items are mostly dumb containers
for a `.selected` bool flag.

Bug: T289560
Change-Id: I6bffda3b74a4bca26032e2602563d64f7bf9bf40
2021-08-30 19:48:42 +00:00
Thiemo Kreuz 926913cad6 Add/simplify assertions in transclusion outline related code
Bug: T289560
Change-Id: Ia76e13582a182c8184b3e8642c5c03bf3d674a24
2021-08-30 14:58:50 +00:00
Thiemo Kreuz 115441b5d8 Add rather complex QUnit tests for transclusion outline code
These are more integration tests than actual "unit" tests. What
the tested code does depends a lot on e.g. how the model and
spec classes behave, and even on some events. Which is good. We
want to cover all of this with tests. The only question is: Is
there a good way to make these tests easier to read, while they
still cover the same code?

Bug: T289560
Change-Id: I8c681f161c272d143a07ca4d0080b4089b48bcb6
2021-08-28 12:34:36 +00:00
Thiemo Kreuz 1f19d85ea8 More test cases for more complex template dialog functionality
Contains:
* Full test coverage (I believe) for the filter functionality in
  …OutlineTemplateWidget.

Also some TODOs for missing tests I believe are critical.

Bug: T289560
Change-Id: I2ac5add8e189d501d3558bbd4854cb92155bcb96
2021-08-28 14:02:57 +02:00
jenkins-bot e778057389 Merge "Remove not needed boilerplate from QUnit tests" 2021-08-27 10:56:08 +00:00
jenkins-bot f2b94e10b6 Merge "More test cases for MWTransclusionOutlineContainerWidget" 2021-08-27 10:00:04 +00:00
jenkins-bot 4dd435f3af Merge "More meaningful assertions for all outline part widgets" 2021-08-27 09:59:21 +00:00
jenkins-bot bdb3a783c9 Merge "Add basic QUnit tests for all transclusion outline classes" 2021-08-27 09:59:16 +00:00
Thiemo Kreuz a0e613d8fc Remove not needed boilerplate from QUnit tests
These tests obviously don't need this extra environment.
They run just fine (and faster) without.

Bug: T289560
Change-Id: Ib186a07cd556f741e0440ffa54ae6aaaf626adcd
2021-08-25 12:01:14 +00:00
Thiemo Kreuz 9392282bf2 More test cases for MWTransclusionOutlineContainerWidget
This also fixes a mistake in the class where we forgot to
disconnect event handlers when an element is removed from
the list. This doesn't have much of a consequence, as the
event flow is only in one direction, from the destroyed
element up. This is not possible any more.

Bug: T289560
Change-Id: I0bcc1d68c50b8cbdb033ef6692b34e2fc94e8d85
2021-08-24 15:46:46 +00:00
Thiemo Kreuz e56b4e82f5 Add test for ve.ui.MWParameterCheckboxInputWidget
This is not a file we created recently, but one we care about.
This is also a nice start to get in the mood to write tests.

Bug: T289560
Change-Id: I6475b00508cfa9188ab0d78c2bfd31bab8aed6ed
2021-08-24 15:44:43 +00:00
Thiemo Kreuz 352e7795ca More meaningful assertions for all outline part widgets
Bug: T289560
Change-Id: I81f882d6e6793198371ff9ff3e1e5bbbec98e0d9
2021-08-24 13:17:16 +02:00
Thiemo Kreuz f6953d4096 Add basic QUnit tests for all transclusion outline classes
This is just the smallest possible boilerplate to get the first
trivial test running. More test cases will be added in the
following patches.

Bug: T289560
Change-Id: I3a4e49a7b9761db00b211e933386bad71d4f0942
2021-08-24 11:31:14 +02:00
Thiemo Kreuz 4f9426d61a Rename …ParameterCheckboxLayout now that it's an actual Widget
This is a direct follow-up for Idc5e048.

Bug: T285323
Change-Id: Ia03d594386a9a59a4819bf78c7d2edcd465dfd52
2021-08-20 09:56:40 +02:00
Thiemo Kreuz d1f87646df Make checkbox element in the sidebar an actual widget
This should not have any effect on how the thing looks and
behaves.
* All elements in the sidebar should be reachable with the tab
  key, including disabled elements.
* Enter jumps to the corresponding paremeter in the content
  area on the right. But enter doesn't change the checkbox
  state.
* Space canges the checkbox.

The class will be renamed in the next patch.

Bug: T285323
Change-Id: Idc5e04828ece0ba77a65e4c839cd3ffccc3b6733
2021-08-11 09:12:00 +02:00
Svantje Lilienthal ee98622076 Change style for adding an "Unknown field" in the parameter search
Bug: T286236
Change-Id: Ibb3df034c9b423dd4130d8242bd7bf0dc742ee2f
2021-07-08 09:20:25 +02:00
Thiemo Kreuz 6757a1bfa9 Character = is not allowed in template parameter names
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
2021-07-04 14:03:21 +02:00
Thiemo Kreuz fecded9f70 Fix parameter search being to relaxed on unknown params
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
2021-07-01 20:09:51 +02:00
WMDE-Fisch c4f59132ae Only add asterisk after word characters in improved template search
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
2021-06-21 21:07:28 +02:00