There is no point in firing this event when noting changed.
This should reduce flickering and some of the issues described
in I97d77f4.
Change-Id: I7c387889a4a33dac5053cec11a0641d358020b56
This just copies the colors from the old sidebar.
* When hovering with the mouse (without click/press) the background
is gray, and the text black. Relevant for readability via WCAG
AAA.
* On click/press the background is blue (slightly darker than a
selection), and the text is dark blue as well.
As noted in
https://docs.google.com/document/d/1V0rXMPr6upNjHF9AkROx4R8IF1LDZUzrG4K6oWT08sU
Change-Id: I443045b55826ef390688b32616dfdcfdc6555eb3
As a reminder (not part of this patch): Pressing enter on the name of
a template should select it, and jump to the content area on the right.
Pressing space (that's what this patch is about) should select as well,
but not move the focus.
The best way to test the behavior is with a multi-part template.
Bug: T285323
Bug: T289043
Change-Id: I97d77f43b231696f92ba6758a6b8feac34e02e6d
I tried to review all of them. Some of the changes I did:
* Make sure the `config` parameter is not marked as optional
when it is not.
* Make sure default values are mentioned.
* List individual `@cfg` options when it makes sense.
Note I don't list all options a class could accept (e.g. via all
its parent classes and mixins). That's too much. Instead I checked
how a class is actually used and list only these options.
Even then I don't list everything, e.g. unspecific options
like "classes" that can be used pretty much everywhere.
Change-Id: Idf4fbe1dc3608ace277df9e385f2f140df3a2f50
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
* The template model fires an "add" event. Listeners don't
automatically steal the focus any more.
* Instead there is a separate "focusTemplateParameterById" event
fired from all relevant places that add a parameter.
* The "remove" doesn't steal the focus any more.
Bug: T285323
Change-Id: I93f17727524bfbcf6f11647a6c2441781337c4cc
The 'classes' property is a OOUI interface. Personally, I like
this code style better.
However. It appears like the code style in this codebase is
somewhat mixed. It looks like the top-level .$element always
uses .addClass(), while other code uses the 'classes' property.
Should we unify this?
Change-Id: I9ecd75e22d00f06ffd707f766dc9e8d748ff9a37
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
When I try to click the input field, the expand/collapse button
is focused instead.
This also fixes a copy paste mistake in this class.
Change-Id: If9ab340711fbe7d88845c008360fde5df7059df0
Before, the content pane (the right half of the dialog) was moved
to the right, outside of the visible viewport. But it was still
active and could i.e. be navigated to via the tab key. Only truly
hiding it solves this issue.
Bug: T274554
Change-Id: I8925a9cca0099528aca8e98452816b5f9dd23a76
The original idea was to make the interface as narrow as
possible. However, it turns out it's better to model the
"templateParameterClick" event more closely after the "choose"
event.
This is split off to make reviewing the following patches
easier.
Change-Id: I271f576c6cd756cecfc6cb1fd64810f8da5c3575
Pure cleanup, doesn't change behavior.
Change If8da5ae85dff63c34 included in OOUI v0.42.0 tracks invisible
controls, so it's no longer necessary to maintain persistent class
variables pointing to the buttons.
Also simplify repeated logic to make it clearly exclusive.
Bug: T290554
Change-Id: If9b6404d7061999540515645fa8e50b9a21f5a21
We want to assert that value is true-ish, and that it doesn't equal a
default or auto string.
Bug: T290554
Change-Id: I454dda8d0085a8d3898a0d5b1a3ecc6dd7c2c9e4
The list of parameters should remove itself from the list of
possible tab navigation targets when it's empty.
Note there is no way to remove elements from the parameter
list. That's why we don't care about "remove" or "clear"
actions.
Change-Id: I8b1215117e0ddc94f787d173e9bea6f7567d9671
Previously, when the tranclusion dialog was being resized, there was
a specific width at which both the sidebar was collapsed and the dialog
was downsized from "larger/large" to "medium". This resulted in the
dialog switching from fullscreen to floating, since the dialog's width
breakpoint for "medium" doesn't match our width breakpoint for mobile.
If the user continued to downsize past this breakpoint, then the dialog
would eventually switch back to fullscreen resulting in this weird
behavior.
A simple way to prevent this is to avoid changing the dialog's size
at all. Since we don't support collapsing the new sidebar (unless
already in fullscreen), we can just leave the dialog at the
"larger/large" size.
Bug: T274554
Change-Id: I5460cdfb1a7ed73fe7957745ba37055c5f66dce1
Calling .getItems() creates a copy. Using .items is possible,
but strictly speaking a private implementation detail.
Change-Id: Id9438faff88cb5ff3973bca4c959e74d9be7e823
* 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
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
Any of these characters results in bad wikitext, when we accept
it in a template parameter name.
Instead of displaying an error message we simply block the
button, as long as the input is not a valid parameter name.
Coming up with a message is not really worth it, I would
argue. Users typically don't have a reason to use any of these
characters. This is super rare. And even if, the behavior of
the widget is not hard to understand, I believe.
The same is done in ve.ui.MWParameterSearchWidget, a little
hidden in the .addResults() method.
Not yet approved by UX. Can be done in demo time.
Bug: T285869
Change-Id: I5576cdfb90411e5fdec93749f72939d31ecd9c56
E.g. avoid calling the rather expensive method multiple times
in a row, if only 1 of the results is needed.
Change-Id: Iff1d2c0892367e927303f6f45d3231e04c045cab
* Use a more specific …-top property, as this is the only thing
we need to overwrite.
* Bring some selectors in a hierarchical order that makes more
sense.
Change-Id: If36db87d83f699fe0a43ac67d439cac42cbb1fa3
* New help text for the case where TemplateData is present, whether
or not it includes a description.
* Remove help text when TemplateData is missing.
Bug: T288465
Change-Id: I0668ccae8eeb5ffffc626e3b7d24c1d7ed99bbed
This just repeats what the base class did. It looks like this
was forgotten when the base class was introduced in I097311e.
Bug: T75822
Change-Id: Ie5f5d358f24be7f9168214ea80713b0f7f295f47
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
I tried to memorize the cursor position in the list of
parameters. This way you could leave the list with tab, and
return to the original position with shift+tab. Unfortunately
this is not how the SelectWidget works. The moment the
highlight is gone it's gone. There is nothing that remembers
a position. We could introduce code to do this. But I feel
like this is a lot of effort for not much benefit. Also not
listed as a requirement anywhere, at the moment.
Bug: T285323
Change-Id: I8d44ba4539ec4b5535bc031accfeacd87e1886eb
This got lost in patch I20dbd2b.
Both events come from the same sidebar class. The difference
between the two is:
* selectPart is when the button representing a top-level part
is clicked.
* focusPart is when a parameter name is clicked while the
parameter is already checked.
Yes, this is confusing at the moment. Following patches will
rename, merge and split a lot of these events to be much more
self-explaining.
Bug: T285323
Change-Id: I0c6b53c93c712ff5e47c1beb5199d590cba7ab1a
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 there are currently two different code paths utilizing two
different events. The existing event handler actually changes
the selection of the top-level part in the sidebar (the
corresponding template name turns blue). The new event handler
highlights a parameter (it turns gray). This is currently
intentional (partly because of a bug in OOUI). I will try to
merge these code paths, if possible.
Please test, and if it works fine from the user's perspective,
please merge it as it is for now.
Bug: T285323
Bug: T289043
Change-Id: I8fafee68b8b7ff225c7b3c327f483f3426d8129c
Proof of concept: while flex is – well – flexible, it feels like
this should be possible with some good old block containers and
margins. It's pixel-perfect in my test.
Bug: T288465
Change-Id: I1458900fff197e08ce318398524a3cf2b6b9ee2a
- Change description text according to ticket
- Make sure link to template page opens in new tab
- Add missing placeholder text
Bug: T272487
Change-Id: Ie8189e9cb9db5908e8fc5fc8bf7ff20df5595094
This fixes a few style issues:
* The buttons that represent top-level elements have a proper
2px focus rectangle again. Back to the OOUI default.
* The list of parameters does have a 1px focus rectangle all
around. Intentionally thin because there is a 2nd level of
keyboard navigation (via cursor keys) in this element.
* All these focus rectangles look the same in Firefox. Before,
it was a thin dotted line on the parameter list.
* Parameters with long names don't wrap on a 2nd line any more.
I believe this was working before but got lost in I92e8fd2.
Bug: T285323
Change-Id: I0229b6395a64a9903335bf96349af70fb20ad047
Method names have been changed in I8fa47ed, assuming these are
private. It looks like some hacks exist out there. Let's make
these peoples life easier.
Change-Id: I63c80761fe06e2f3a4bb104fe3e8c17d1c7faa02
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
When I press the button to expand the input field for
undocumented parameters, it needs to be focused. Otherwise I
have to click it manually all the time.
We probably forgot to list this as an acceptance criteria when
working on Ic5dcd36.
This also replaced a bit of JavaScript with CSS. I do this
mainly because I found the mixture before (one piece was
hidden via JavaScript, another via CSS) a bit confusing.
Bug: T272487
Change-Id: I0cbee63c65a37f2f1860bde007c1e5c8408ba006
This makes sure the corresponding top-leve part is selected in
the list on the left when navigating the main area on the
right.
Bug: T289043
Change-Id: Id1b398e1786c4099d5b14fe88dd21a106269096b
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
This comes with a few significant changes:
* A whole bunch of places in the code that focus and highlight
an element in the old sidebar consider the new sidebar now.
* Same when e.g. the toolbar at the bottom needs to know which
part is selected. This is read from the new sidebar now.
* To make this possible I had to merge the small helper class
we introduced in I7bc73cc back into the dialog.
It's helpful to understand how the event flow works:
* You click a template name. This does nothing (does not select
the element). It only triggers an event.
* The event is catched by the outer container that manages
all parts. From there all elements are unselected, and one
selected. This call is internal and should not trigger
another event.
Bug: T285323
Bug: T288827
Bug: T289043
Change-Id: I4a2d2b83cf2691423d4b0e6f4487228fa3c7b56d
This is mostly, if not exclusively visual, at the moment. The
actual state is still managed by the old sidebar.
I made the element OptionWidgets for convenience. This gives us
all the functionality we need (primarily setSelected and
isSelected), without to much clutter. However, I didn't made
the container a SelectWidget. This comes with to much stuff we
don't need at this level, e.g. cursor key navigation.
Bug: T285323
Bug: T289043
Change-Id: I20dbd2ba23ceaa9125947b25e037c0bb3c91a471
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
Similar code elsewhere checks whether this.root is set
(e.g. ve.ce.FocusableNode.prototype.onFocusableSetup).
And code here checks whether this.surface is set.
Bug: T289201
Change-Id: If07dc75ca76f2d171bc2eae83be10083d95096f8
Most notably:
* Move some code snippets from the outer …TemplateWidget to
the inner …SelectWidget, without introducing new
dependencies.
* Move all knowledge about the item class
…OutlineParameterWidget class into …SelectWidget.
* Some more self-documenting method names for event handlers.
* Avoid the somewhat ambiguous variable name "checkbox" in
favor of "item". That's how it's named in the upstream OOUI
…SelectWidget.
This is extracted from the following patch Ibd94c39. The
difference is that the following patch adds a new dependency:
The …SelectWidget gets to know the template model. This patch
here contains all changes that are possible without this new
dependency.
Bug: T288827
Change-Id: I187f313c84424b28005d9276cb1356029f9ebb75
There are 2 situations:
1. Either the template name is used in a [[…]] link. In this case
we must provide the namespace. MWTemplateModel.getTitle() does
this. However, it's not a mw.Title object and therefor not really
guaranteed to be a valid title. This is fine. The worst thing
that can happen is that the link points to an error message.
But this should be entirely unreachable anyway.
2. Some messages want to display the name of the template.
Ideally without the namespace. That's what
MWTemplateSpecModel.getLabel() is for. Again this is not
guaranteed to be a valid mw.Title. But it doesn't need to. It's
only used as a label.
Change-Id: I03d0481201620a2f5c444ee32b656bcaade98aac
We should only need that label for the link. The other mechanic
would fail when editing wikitext like this:
{{{{echo|<}}|param=foo}}
Bug: T272487
Change-Id: If8d228b40bf1589181e83e8f68f3c33b4c7759c7
It appears like it's currently not possible to reach this code
with an invalid template name like `{{foo}}`. But this is not
guaranteed.
The purpose of this code is to call the TemplateData API. This is
pointless when a title is invalid. We know a page with this name
can't exist. So we skip it.
But that's all this code cares about. It should not crash. Nor
does it need to report this situation.
This is related to the discussion in Ic364e75.
Change-Id: If9bacc91b1c7bb110b33bfd395e1cbdf538e6c22
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
