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
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
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
The code in .cacheTemplateDataApiResponse() where the `specCache`
is filled skips missing pages. .setTemplateData() is never called.
While we could – in theory – check the `missing` flag (as done in
patchset 1), this flag never makes it to the spec.
Rather simple solution: Mark everything as undocumented, as long
as .setTemplateData() is not called.
This affects only missing pages. .setTemplateData() is called in
all other situations.
Bug: T272487
Bug: T276574
Bug: T290136
Change-Id: I7045e84f2f2ba5aa4591c94ea495b0249e6c40d6
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
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
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
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
Not only do we want to make sure getUniquePartId() always starts
at 0 and increments correctly, it should return a number (and
not e.g. "part_0").
I realize the getTitle() test is also testing functionality from
mw.libs.ve.… (can be found in the file ve.utils.parsoid.js).
This is intentional. What we care about at this point is not a
library but the very specific functionality of a very specific
method we use quite a lot in code we touch.
Bug: T289560
Change-Id: I43c1d00dacf27a68b16f62ecca4adda22f437391
These tests obviously don't need this extra environment.
They run just fine (and faster) without.
Bug: T289560
Change-Id: Ib186a07cd556f741e0440ffa54ae6aaaf626adcd
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
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
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
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
This reverts commit 0d4dee341b.
Reason for revert: This made it entirely impossible to add a
deprecated parameter, even if done intentionally. Needs more tests.
Bug: T274551
Change-Id: I7389bad0845cd1ce78f9d7ef71592cb1ce2a063e
Separation of concerns:
* The template model knows which parameters are currently used,
but doesn't know what's documented.
* The spec knows what's documented, but doesn't know what's
currently used.
Change-Id: I97cac00d6775a17a07059d0e8a7a116adc6080b3
There are some methods that behave different when a parameter
placeholder (where the parameter name is "") is present. Some
skip placeholders, some don't. This is critical to cover
before we make further changes to this class.
What I also do in this patch:
* Use shorter variable names to make the code easier to read.
* Don't reuse the `transclusionData` variable but use a copy
of the expected value. This makes the assertions much
easier to understand.
* Bring every test in the same "setup" → "execute" → "assert"
order.
Change-Id: I41a691c56bc509b132dc719ff820ae1ade4ccc3a
A lot of the checks are redundant. The first check still is
redundant because the later two cover everything as well. But
I left it for performance reasons.
Additionally:
* There was no test for the method.
* This patch also updates a few pieces of documentation in the
same class.
Change-Id: I10f2944a844cc070bdc08dec6719929b383e34fa
