Commit graph

156 commits

Author SHA1 Message Date
Thiemo Kreuz d5857c20f5 Remove invisible stuff from BookletLayout OutlineItems
Technically the old BookletLayout sidebar is still there. But it's
never visible, effectively dead, and meant to be removed. This patch
just empties the OutlineItems of all template dialog related pages
without actually disabling the old sidebar.

Note this partly reverts Ie57f462.

Bug: T310859
Bug: T310866
Change-Id: Ic0b7d703f369045ed342426563f8eeb3e47046db
2022-06-20 09:31:53 +00:00
Thiemo Kreuz 218ac78960 Remove obsolete "search for a parameter" page and widgets
… as well as messages, CSS, and icons that are now unused.

Bug: T310859
Change-Id: I1a2797728788cf1a86299414c1430ded0b362d4e
2022-06-17 10:23:00 +02:00
Adam Wight 1b4c81c27d Remove $wgVisualEditorTransclusionDialogInlineDescriptions feature flag
The feature is always enabled now.

Bug: T307188
Change-Id: Id8b39d44901dee9118837fc4670d7faa4ef19b43
2022-06-15 12:09:47 +02:00
jenkins-bot b99ed46e1b Merge "Improve test coverage of template dialog outline" 2022-06-13 12:24:01 +00:00
jenkins-bot 24ec9b2783 Merge "Add prefixsearch fallback test for MWTemplateTitleInputWidget" 2022-06-13 12:12:09 +00:00
Adam Wight 23f0bdf822 Remove $wgVisualEditorTransclusionDialogSuggestedValues feature flag
Feature is always enabled now.

Bug: T307188
Change-Id: I6f8288d31fecc6827a7a9e5e9705239e15c86ae2
2022-06-10 16:40:06 +02:00
Thiemo Kreuz 9297428eca Improve test coverage of template dialog outline
This focuses on some scenarios that are
a) complex enough to be worth a test,
b) but simple enough so I don't need to spend hours on comming up
   with a test setup. ;-)

This patch also simplifies the ARIA related code in
MWTransclusionOutlinePartWidget a bit.
* Check 1 of the 3 ARIA configs only. Only having one is already
  helpful and should not be skipped.
* No need for the large conditional. setAriaDescribedBy() works fine
  with undefined.

Bug: T291157
Change-Id: I142782ec9b96147de64497f4f6a373eae05b9c8e
2022-05-04 12:01:58 +02:00
Thiemo Kreuz 2cd08e42d7 Add prefixsearch fallback test for MWTemplateTitleInputWidget
The idea of the code under test is that it combines the results from
2 different API requests in a specific way. This is a first basic test
where both search results are small.

Bug: T291158
Change-Id: Ic4de57fb6b85afb952ea604769fddd06d44814c0
2022-05-04 07:11:15 +00:00
jenkins-bot 174c7a39f6 Merge "tests: Use ve.test.utils.newMwEnvironment()" 2022-03-10 22:51:26 +00:00
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
Bartosz Dziewoński 94633fd165 Use $.parseHTML() in tests where appropriate
It removes some indirection.

* Find:          \$\( '<.+?' \)\.toArray\(\)
* Replace with:  $.parseHTML( \1 )

(also replaced a few by hand that weren't caught by that regexp)

Change-Id: I2639cc4a26bc960046a189504dd8058344e14d60
2022-03-09 01:07:57 +01:00
Ed Sanders 24973a2928 Update VE core submodule to master (15c1efdc2)
New changes:
cf7b2a9c4 Restore margin between wrapper paragraphs and document edge
629109b6b ve.ce.Surface tests: Sometimes assert originalDomElements in paste tests
3a5bd323c Use wrapper paragraphs in fixUpInsertion

Local changes:
* Update tests to ignore some internal properties

Bug: T299815
Bug: T299817
Change-Id: Ie52348f62320eb135c2f950ac78a77d9c665fef2
2022-03-09 01:05:15 +01: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
Bartosz Dziewoński 8c12259900 Update VE core submodule to f5c7f0f204
New changes:
e5963f084 DiffElement: Remove doc-child-change wrappers
90a359f79 Follow-up Ia593dad8: Use modifyData when sanitizing
f5c7f0f20 Localisation updates from https://translatewiki.net.

Local changes:
* Update tests for the removal of doc-child-change wrappers

Change-Id: Ib29f23625de9151d50e0f3b7796b15ca9ed89133
2022-01-24 20:13:21 +00: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
Ed Sanders 9e4915d7c3 Tests: Fix unbalanced HTML
Change-Id: I997c68ce33263004b8ae9b686aae7295eec12ea7
2022-01-04 02:01:38 +00:00
jenkins-bot 6dc75d8283 Merge "Remove obsolete performance bottleneck from TransclusionModel" 2021-12-17 13:24:19 +00:00
Thiemo Kreuz 7cf10ea474 Delay initialization of all invisible "add parameter" components
The "Add parameter" page always starts collapsed. Even if a template
doesn't contain anything but this. But most of the content isn't
visible, unless the user presses the button. It's not only a lot of
content, it's also rather expensive, including .parseDom(),
LinkCache.styleElement(), and ve.targetLinksToNewWindow(). This adds
up in large multi-part transclusions. In an example with 200 parts
the total blocking time goes down from 2.9s to 2.4s. Which means this
is not a major bottleneck, but still worth it.

Bug: T296335
Change-Id: Ieab9fd35d145142b04d2267d8e5a2e10a4c02784
2021-12-15 08:28:16 +00: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
Adam Wight 1142ca3ce0 Skip flaky test for now
Bug: T292318
Change-Id: I03437d33f26c48208b84a578ff185bff6f9034a7
2021-10-07 15:00:14 +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
jenkins-bot abd115c054 Merge "First QUnit test for dialog classes" 2021-09-29 11:57:30 +00:00
Adam Wight d70941593d First QUnit test for dialog classes
Wrote a first test, that checks the parameter checkboxes showing up.

Bug: T291155
Change-Id: Ie79272442a0d0532607fe93e9ea538635812e671
2021-09-29 10:00:34 +02:00
jenkins-bot 8b539794a7 Merge "Cover suggestedvalues feature in MWParameterPage class" 2021-09-28 16:29:08 +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 33b4d0dfc6 Cover suggestedvalues feature in MWParameterPage class
… as well as improve the coverage of MWAddParameterPage a bit.

Bug: T291156
Change-Id: Ibacb602298e5200b00a23092e5b9b211c43d6719
2021-09-27 18:22:10 +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
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
Andrew Kostka a38338259d Improve input validation for the add parameter page
This patch improves the error handling for when a user tries to add
a parameter which is either an alias of a existing parameter, the
primary name of a existing aliased parameter, or a name/alias of an
existing parameter which is shown with an override label.

The error message was modified to always refer to the conflicting
parameter using the same name that is has in the sidebar.

Example: A parameter named "Parameter B" is already present in the
sidebar under its alias "B". When a user tries to add "Parameter B",
the new error message will inform the user that the parameter they
are trying to add already exists as "B".

Bug: T285869
Change-Id: I762b72b6cf14eb8ff5fcef63b4dcb70e297050de
2021-09-13 16:58:11 +02: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
jenkins-bot 0cfd298d18 Merge "Fix and test page name splitting code in template dialog" 2021-09-13 12:23:17 +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 3b59d7ce0d Add tests for ve.ui.MWParameterPage class
Change-Id: Ie09ef4745c006822cc66e845cb16f54d62fa3b7e
2021-09-11 17:39:17 +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
Andrew Kostka 4c0666fd19 Don't allow users to add parameters that contain forbidden chars
Bug: T285869
Change-Id: I1011949c2724939f3cec1e1a2ae1c821c33eff84
2021-09-09 11:43:49 +02:00