Note how the two-pane layout was already handling the two events
identical:
sidebarPartSelected: 'onSidebarItemSelected',
templateParameterSelected: 'onSidebarItemSelected'
We move this knowledge up one level into the sidebar. Both actions
now trigger the same event.
This implies that we are now using the event formerly known as
"sidebarPartSelected" to act on both top-level parts as well as
parameters. We reflect this by renaming it to "sidebarItemSelected".
Change-Id: I9a2c95f91f05de7312d38ec4c8b360141a0c447d
This is not only fired when a parameter is added. It's also fired
when pressing spacebar on a parameter that already exists.
Change-Id: I245aa9f5938eb38c3a3f224a4d642d57068cf23b
This is not only fired when a parameter is checked or unchecked.
It's also not only fired when another parameter in the parameter
SelectWidget is selected. It's always fired just because the
spacebar is pressed. This is so we can scroll that parameter into
view.
Change-Id: Id621405b7ca3116cd4a06f474e49776d0830dccc
This regression from Iaf089f4b271fd853b17c1aa7f5938510ea8f5431.
Not calling the parent here was is essential, because when clicking
the checkboxes this event will trigger the focus of the whole widget.
That's not what we want in this case though. Also it seems we don't
need any of the logic from the parent and can get rid of that
complexity.
Bug: T312855
Bug: T312856
Bug: T312924
Change-Id: Ib3a2eeeb6d519dfa1bb627049f6b3a4708017e86
Triggers scroll in all situations where highlighting changes.
Including keyboard navigation.
Bug: T312542
Change-Id: I849f64c5cefe0cac3fde6e848b23b7b0cfc489ce
Introducing a set method to have a different state for a set
parameter and a highlighted one in the selection.
Allows us to remove a lot of workarounds for missusing the
highlight state and fixes several issues with these workarounds.
Main implications:
- Keyboard navigation and mouse hover now sets the grey highlight
- If a parameter is set (blue highlight) keyboard navigation returns
when focusing the SelectWidget
- If nothing is set keyboard navigation starts at the top after focus
- Unchecking a parameter using space will not influence the keyboard
focus in the list
- Highlighting a parameter with the mouse lets keyboard navigation
continue from there.
Bug: T312647
Bug: T311204
Bug: T312213
Depends-On: I385dca1d95033961d3844e888521750443e49c95
Change-Id: Iaf089f4b271fd853b17c1aa7f5938510ea8f5431
This prevents an invisible and unactionable focus target when the
parameter search field doesn't match anything.
Bug: T312547
Change-Id: Ib29913a547cd68649d29e28d921c4c1358bad7b8
We don't want to restore selection. Instead, we'll remove all
parameter selections onFocus, in a later patch.
This reverts commit 787d44af66.
Bug: T312017
Change-Id: Ia1dc8061dfd1813a58befff5adc5c3882b54d8e2
This should be safe because selection is only set from use cases
in which we do want the sidebar scrolled to the matching item.
Bug: T290975
Change-Id: Ib0c66bb048768d633d0f638c775eba24cd652db8
When blurring out of keyboard parameter navigation, the
last-selected parameter will be highlighted again.
Bug: T312017
Change-Id: I8b0fb667b44b324529d4c45c39bf21573517f989
Tabbing into the parameter select list should highlight the first
parameter, but focusing a checkbox by clicking it to uncheck should
not trigger this highlighting.
Copied from the superclass method.
Bug: T312014
Change-Id: Ia1fd450bad4861eb2815ca21eae69ee31e40ac08
New sidebar events take a `soft` parameter allowing reuse between
"hard" and "soft" selection and parameter add. The "hard" variant
causes a content-pane focus.
Bug: T311987
Change-Id: Ic49718840ae56eb4cfab01ce964a2fbc2d8db63b
If nothing has been selected, the action buttons should be
initialized in a disabled state.
Bug: T311608
Change-Id: I32fee0d73f6e13e09dc421c944df73b79e0260bb
It seems that the controls widget was the only side-effect we still
needed for maintaining existing behavior.
Bug: T310866
Change-Id: I507dacef4e56946b836b0fca31effce611260aec
This patch does a few closely related things:
* Replace a direct dialog → sidebar access with dialog → layout →
sidebar.
* Move the misplaced removeButton.toggle() to a more appropriate
place.
Bug: T311069
Change-Id: I5a2802aab587a6f7de4681bce4e9961a064ef8ee
Value changes are triggerd and tracked from the parameter page and
change events will be forwared to the outline, if they are relevant.
Initial value is read from the model.
Bug: T308730
Change-Id: I0a3b0faf40aee44889404dcce31d850714360580
Removing a class name that's going to be deleted from places where
it's not strictly needed, e.g. comments.
Bug: T307188
Change-Id: Ifb14695d05510d2c0e25623afa99c4e84af3aaf9
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
Will be removed when parameters are added. Needs different margins
depending on beeing shown in the single transclusion mode without
header or on multiple transclusions.
Bug: T300710
Change-Id: Ieb95d7276aa4d4b0fcbb74f87ab734e4a393dc21
Before, the fallback algorithm was somewhat adaptive, trying harder and
harder the fewer CirrusSearch results have been found. This updated
algorithm guarantees that 10 results are shown.
Warning: You might see only 9 results. The reason is a bogus, unrelated
behavior in the mw.widgets.TitleWidget in core that's used as a base
class here. There is a "showMissing" option that's apparently enabled
by default, and prepends a non-existing title from the main namespace,
ignoring the "namespace" option. This extra result from the wrong
namespace is later dropped by the very same widget.
This code here sees 10 results before the bogus one is dropped.
Disabling "showMissing" causes other issues. We would need a series (?)
of custom hacks to work around all this, but this seems inappropriate.
Let's live with 9 for the moment.
Bug: T303524
Change-Id: I2c577c9ef2752b6c6cd360f4023e151e9272fcd5
The main advantage of this change is that it drops the assumption
that the index starts with 1. This is not necessarily the case when
we prepend extra search results. Dropping this assumption here allows
to simplify such code.
* The incoming list of pages is guaranteed to be an array.
* There is no point (any more) that could cause the array to become
sparse.
* Note we still make a copy of the `origPages` array at some point,
e.g. on `.filter()`.
Bug: T303524
Change-Id: Ifbd92bb052155c613d2ca21ab6d54a0b3ef28c0c
This option is not only buggy (it just doesn't work when the namespace
option is set the same time), it is not useful in this context even
if it would work. It doesn't make much sense to suggest non-existing
templates in the context of the template dialog. If adding a
non-existing template really is what the user wants, they can still do
this by simply typing the name of the template and submitting the form.
We never need this to show up in the suggester below the input field.
The main advantage of this change is that is saves 1 useless API
request that's potentially done every time a key is pressed.
Bug: T303524
Change-Id: I903340a06d6e6490bb58f628f41903aa044ccb21
This separates the two steps:
1. See if items in the list of `origPages` miss their `.index`
property, and add it if possible.
2. Later code doesn't need to care about redirects any more.
Note that `origPages` is not used for anything else. And even if,
it's not wrong to have the index for each search result on both the
redirect and the redirect target.
Change-Id: I12135f0430c944b4e33c49ece7779d7c3bb6c211
This is not a pageid, but a simple numeric index in an array. Luckily
it doesn't make a practical difference for this particluar way of
iterating something.
Change-Id: I7ec9ace00d4fba7adde17670058a0365b30f5617
The result is guaranteed to be in formatversion=2, where the list of
pages is an array, not an object.
Change-Id: Ic73a68c3e249a70108a6a19a89f4ff6c475794ed
This code is optimized for the 2 most relevant use cases:
1. When Cirrus finds 10 results, we still want to search for the top 1
prefix match. This is critical for templates like !!. This will appear
at the top. unshiftPages() makes sure the limit of 10 is enforced.
2. When Cirrus fails to find anything, we search for 10 prefix matches
and use these instead.
The code can also handle everything in between. For example, when
Cirrus finds 5 results, we search for 5 more prefix matches and add
them when Cirrus missed them. The total number in the end might be 5 to
10 depending on the number of duplicates. This is intentional. Why?
Let's say we always search for 10 prefix matches and add them to the
top when Cirrus missed them. This might remove _all_ Cirrus results.
This shouldn't happen. This extra code is only to fill in glaring gaps,
not to replace Cirrus. 5 results are fine.
Bug: T303524
Change-Id: Ib0471795124c0c7001b6901edaf8e7b380e426b1
There is already some kind of "fallback" to prefixsearch. We always
check if the top-1 prefixsearch result is part of the result set.
Because of this the current worst-case scenario is that only this
1 result is shown.
This patch implements a full fallback to prefixsearch. But only when
there are 0 CirrusSearch results. Further tuning might be done in
later patches.
Bug: T304925
Change-Id: I1927eedad60c9b9ac2021481a85376c08ccf6fdb
In detail:
* Just compare strings where possible. Sometimes the first character
alone is distinct enough.
* Add missing ^ anchor to avoid possibly running unbound pattern
searches. This is not strictly necessary when we know the input is
fixed-length. Still I find it more robust and more obvious what it's
meant to do. Not to "search for some match somewhere" but to check if
the input _is_ a match.
Change-Id: I9b7651d8ec1266521c22b38ccabb95a947903f72
.test() is the dedicated syntax for a boolean "does match? yes/no?"
check. .match() returns an array of matches, or null. This is just not
needed in these situations.
Change-Id: Ibb996ab843d1a6c7d7af98d6a112990665d543b2
We ended mixing two concepts in a single method:
* We need a method that allows us to create the parameter list widget
when it's needed, even if it's empty. This is relevant when a template
is entirely empty, and the first parameter is added. This wasn't
working. Instead the parameter list was created with all parameters.
* On initialization we either want all parameters to be shown, or only
used ones. But this code is only needed once, on initialization. I
ended inlining this code in this patch.
Bug: T300640
Bug: T304046
Change-Id: I6620a870e4420dcb8fecf522b3274458eeec891d
Due to the stickyness we need to scroll the header and the first
visible item from the parameter list to have the best result in all
cases.
We also only want to scroll when the user triggered hiding the
fields. Not when the template outline view is initialized.
Bug: T302965
Change-Id: I84d293888a7dbf13ec655c293c0fc3a79edca698
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
Same random finds while working on something else. I carefully
checked and made sure these methods are actually called without the
optional parameter.
Change-Id: Iab36fd130258322985b5d6e7f8e1f7b4ee235ba2
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
The `.` character class matches any character *except* line terminators,
but edit summaries can contain those. Use [^] to match truly everything
in the comment part. (In the section part, I assume `.` is okay.)
Bug: T302103
Change-Id: I29fcdd7489d118674bab5cfe5c0a15b8e4efac64
I bit more logic was needed to make sure, the state of the widget
resets when all unused fields are added during search.
Bug: T299811
Change-Id: I3006c233fda5490e323bc3a3e631bf0c1199bda3
Covers the base functionality to hide the parameters. Performance
optimizations could be done in a follow up.
Bug: T300640
Change-Id: Ia99b5da392273f1445e475a0720a656460612dcf
The behavior is now consistant with what would happen when the
buttons are triggered.
Instead of emitting a button click I directly wired the methods
that will by triggerd by the click. This might make it easier to
remove the old sidbar later.
To avoid movement when the buttons should be disabled, an
additional check was added to the onMove method. It's not identical
to the more complex check in the outlineControlsWidget, but should
be enough for our use case. The onDelete method already just does
nothing if nothing is selected.
Bug: T300971
Change-Id: I8a278c9657c91fd648944b5a8c1204c9fff75b7e
This option was added in 0.43.0. Now that the close button is handled,
the remaining functionality (store a flag in local storage, and fixing
link targets) doesn't really justify a separate class, especially as
it's currently only used once.
Change-Id: I0fd81cadccc077dbf957302f9f41409c5a1f4f20
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
From the user's perspective nothing changes. A template without
parameters doesn't show anything. (Technically there was an empty
<div>, but it doesn't do anything.) The moment the first parameter
is added the required sub-widget is created and available from there
on.
This saves loading time and memory, especially when a multi-part
template contains many templates without parameters.
Bug: T298259
Change-Id: Ib1bd2cd0be4fece4acc92e6e5f63133a7986cf81
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
The search field will stay at the top of the window while scrolling a
single template. In multi-part transclusions, the header will also
be sticky.
Hides the template header in single-part transclusions.
Bug: T298618
Change-Id: Ib050e30a50ef965c1524e977d3a600c3ff836774
This is guaranteed via ve.init.mw.Target.getContentApi(). But the
ContentTranslation extension replaces this, and does not set a
formatversion. See e.g. SectionTranslationTarget.getContentApi().
Bug: T298599
Change-Id: I8768cae3153e9cbc29a8796ec21ef249f80471ed
Prevents accidentally treating plain text or user input
as HTML, which could be an XSS vulnerability.
Change-Id: Id4af48447a0907962a57340cb60aca08df9cc505
* Fix incorrect use of .append() instead of .text() (which was causing
some l10n messages to be treated as raw HTML)
* Avoid escaping and parsing HTML several times when plain text was
intended
* Remove some unused options and variables
Follow-up to 839b64d882.
Change-Id: I124257c73fe09713afefccdec8e90200e6ae433d
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
Allows setting aria labels and descriptions on elements in a
convinient way. I did not use the the .mixin. convention here for
because there's already another mixin in that folder that's also
not having .mixin. as part of its name. And then there's also no
no need to open up that extra namespace here.
If we move this upstream at some point this can be changed though.
Bug: T291284
Change-Id: I1b3d40400d539f851f13719e16ced200968a7f92
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
ARIA-selected only works on specific elements/roles. I tried several
combinations here, the most fitting seemed to be the option role, but
that role did not work very well in FF with NVDA. It also should only
be used as direct child to a listbox e.g. with several children.
The next role that's working with ARIA-selected that seemed fitting
is the gridcell. It's still a bit hacky but works well in IE and FF
with NVDA. I suspect that that's pretty good coverage already then.
Bug: T291284
Change-Id: I85c865b0ab12d3923e472e5f36b5c07b7c722180
The following keyboard combinations are now supported on MacOS:
1) CMD + DELETE
2) CMD + DEL
3) CMD + FN + DELETE
(1) for keyboards without a DEL key use DELETE/BACKSPACE
(2) for keyboards with a DEL key
(3) FN + DELETE is a built-in an alias for DEL
Bug: T294519
Change-Id: I896837954b2f0b36a25484080e57d929a5abf774
We're mainly interested in the layout of a button here. From a
semantic perspective this is just a header to an editable area
and not a button.
Bug: T291284
Change-Id: I683cca2e7d6549e652bd03ae1e46f4eff8c07d0a
This is a more radical change, compared to the previous patch.
I will post more detailled explanations as comments on Gerrit.
Change-Id: I6909b3f0b2c153b7ee9995441e995ffa793eab40
The event "focusTemplateParameterById" targets the right side of the
dialog. The input field for the parameter should be focussed. This
doesn't make any sense when the parameter was just removed (i.e.
unselected).
Change-Id: Ie75b1edaebe9d0444b98e66cb56a5c7774393bea
Some cleanup to improve readability and reduce the amount of code.
Relevant bits:
* One method name was wrong. It can actually return parameter ids,
not only top-level part ids.
* I got rid of some fail-safe checks that are never needed or moved
them to a more central place.
Change-Id: I08f2ad7bc7d3f985d6189dff170dda554f3d37c2
The idea of this piece of code is to make sure both sides of the
dialog show the same element. But it doesn't make sense to force the
*header* of a template into view when I clicked on a *parameter*.
Bug: T292718
Change-Id: I9945f8e54c856152f05bf717e43468ab5ab24d2f
I run into this in some local test. There are two reasons this code
can be reached:
* When a wiki doesn't have the TemplateData extension, the
additional API call from line #154 will fail. But the original
search query succeeded. We have the `originalResponse` and can
return it. This makes the code behave as if the additional
TemplateData API call was never done.
* But what if the original search query failed? We still end in
line #183 – as we should. But this time it can't return anything
but undefined. This will be considered a valid, successful API
response. But it isn't.
There might be a better way to clean up this chain of promises.
This is the smallest fix I found.
Change-Id: I02d3d053156da222ee424382007621f314777015
Instead of using an object mapping namespace ids to if they have
subpages enabled or not, pass an array of the namespaces where
subpages are enabled, reducing the size of the configuration that
gets loaded on all requests. Only requires a minor update to the
JavaScript that uses the value (check for array index instead of
object value).
Bug: T291729
Change-Id: Ia0ecac71721eceed52cc90f39ecc560bdf1b7f9b
I moved some code around and found that quite a lot of code wants to
know:
* Is the length of this transclusion exactly 1?
* I need that 1st part.
There is more that can potentially moved from the dialog to the model.
But I don't want to make this patch to big.
Bug: T292371
Change-Id: Ia94ed0450d04dd97c4c41f5bf7c266f9a534e821
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
Note this patch is somewhat incomplete. The feature fully works and
I would like to see this patch merged as it is. But whenever you
press one of the keys the focus is stolen by some element on the
right side of the dialog. This makes it impossible to e.g. press
Ctrl+Shift+Down multiple times. The idea is to work on this in the
next patch.
Bug: T290262
Change-Id: Ic67f2a696c94f1e5c71134d681161221aecbfdf6
… obviously only to methods that are meant to be private, i.e.
only called from within the class (and possibly tests).
Change-Id: I581558078dc7210abac5f5724f71316ac45745e6
The .onUpdateOutlineControlButtons() method doesn't describe what it
actually does. This issue was introduced in I9c5478a. (Intentionally,
to not make the patch to complicated.) Let's continue to rename
things to be a) unique and b) honest about what they do.
This is an alternative to I8d98e61.
Bug: T289043
Change-Id: I4d52ffa6e9e5df2025a0c33031c1517bcb421279
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
This prevents multiple highlighting, which was possible to achieve by clicking
sequentially in the input fields of two parameters from different templates.
Change-Id: I404936f1569ab544b693a9bc6921381636ea8f40
