Commit graph

129 commits

Author SHA1 Message Date
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
Andrew Kostka 075ca72fe8 Add input validation to the add parameter page
Bug: T285869
Change-Id: Iebb982e95aa19bd61fcda915981d505cc243c4b2
2021-09-09 11:42:53 +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 7edb3a82b8 Rename ambiguous onParameterInput event handler
This is split from patch Iebb982e to make it easier to review.

The name is rather ambiguous. Does "input" refer to the input
element? Is it triggered for every key press, i.e. when the
input changes? Or when it's submitted?

Change-Id: Iddbe3bfb9faf3561d8d71b96ffae507799827a95
2021-09-07 09:04:33 +02:00
jenkins-bot 836ab09a79 Merge "Rename and document select/focus events in template dialog" 2021-09-03 09:38:21 +00:00
jenkins-bot 52ed6367f9 Merge "Mark link to non-existing template page red" 2021-09-02 10:25:54 +00:00
Thiemo Kreuz a395e76653 Mark link to non-existing template page red
Bug: T272487
Change-Id: I15b377feda3a4e3b4570986668dfaa066afd70e9
2021-09-02 10:02:57 +00: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
jenkins-bot ef486588ce Merge "Add tests for new ve.ui.MWAddParameterPage" 2021-08-31 10:55:16 +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 e9bd350f68 Add tests for new ve.ui.MWAddParameterPage
The class was added via Ic5dcd36 just a few days ago.

Bug: T289560
Change-Id: I3f729fb6c5d7c28a221d88294d5547809f10a17d
2021-08-30 13:02:52 +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