In the mobile view, parameters don't have left padding so some styles
should only be applied to the desktop stylesheet.
Bug: T304167
Change-Id: I1846512c21aae36f212fe142b7d96ac91e46854b
Currently in VE desktop, items which can be deleted with
delete/backspace do not have this button. If that situation is
considered a problem it sould be solved consitently for all
focusable nodes as part of a wider fix.
Adding a delete button to template contexts makes them inconsistent
with other focusable nodes.
Bug: T274263
Change-Id: I2f7508a605852274ba8f40b2afd1dfd56600aa36
This fixes a styling issue with inline descriptions, for the scenario
when the inline description feature is enabled but the new sidebar is
disabled.
Bug: T304167
Change-Id: Ida4da4605da5143de2a27725d87d5876aea7065c
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
* I can't guarantee this fixes T301914. But I suspect the bug can not
be triggered any more with this plus If9b6050 in place.
* I wasn't able to reproduce the issue locally. But I learned that
1. it's related to the hotkey, and
2. it's because the dialog looses focus, and the focus ends on the
VE surface in te background. Pressing Esc there closes VE. That's
intended behavior.
* I tried to use .trigger( 'click' ), but it doesn't make much of a
difference. The only magic check is if the button is disabled. But no
visibility checks are done.
Bug: T301914
Change-Id: I2f66fc2411144c60cd08baae58452d336b4e9802
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
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
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
Parsing it in the RL module caused the module cache key to depend
on the parse, which is slow and makes ResourceLoader sad. The usual
approach for solving this (I206bb05d28) can't be used, because of
how EditPage generates this message.
Bonus #1:
Generate the message for the correct page title. MediaWiki allows
customizing it per-namespace or even per-title, which we haven't
supported before.
Bonus #2:
Pass the context for message localisation (depends on I5f7c77970d).
EditPage::getCopyrightWarning() was parsing messages without the
interface flag, causing some needless processing elsewhere.
Depends-On: I5f7c77970d0525c0ff394f8bd72c69dcb5d00623
Bug: T298822
Change-Id: Iaa626f0e6379a5a370f9c465cea8528bb5bde7f7
Do show a confirmation when editing templates without parameters,
but don't show the confirmation when you're only on the template search
page.
Bug: T299394
Change-Id: I8205bb6a0f92b7b535a80f7f6a43674f1aa5b51b
We were using addItems() incorrectly, and it stopped working after
change Ib1241f93d214e8a7d2f757a120f84de3879575af in OOUI.
Bug: T299818
Change-Id: I2d00cb929673a419789ca6f8bc308721373e1da8
The only places where this dialog is used now will have the same
button labels and format. We want to use "normal" destructive here
so no additional "primary" styling.
It seems that the focus on the action will only be applied
automatically when it is primary. So extra code is needed.
Bug: T299647
Change-Id: Ib5250b79e85d27ea197b83c6380863d0749e5d89
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
For most nodes, isDeletable() returns false on desktop and true on
mobile. Change e0bbf0b979 meant to
change the desktop version to depend on the new config settings, but
accidentally changed mobile too.
Bug: T298828
Change-Id: I8ec5e8031a27eee18c16dfcd267937cc76722c50
Double clicks have been tracked as "window-open-from-command", but are
now tracked as "window-open-from-context". This patch changes this
back.
Bug: T297773
Change-Id: Icabc5db526c35d681d077043c54ca009b2296bbe
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
The checkboxes are actually indented a bit more than the search box
with this change the extra indentation is applied and the hide
button text is alligned accordingly.
Bug: T298259
Change-Id: I5508ab883c23e7285c023dc127529ffa4dbe58c7
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
Reuse the back button confirmation dialog for the close button. The
condition is slightly different: need confirmation if there are any
manually-entered values for any parameter AND the user has edited
the template in this session.
The "reset" action was synthetic, only used internally and not
connected to buttons or menus. Canonically, action='' is the close
action for OOUI.
Bug: T297792
Change-Id: I4ff644c7ab24ed9ba1a4c27d762563c5d6771cfc
Without changing behavior, consolidate the logic for detecting
whether the editor has made changes to the template. This is
responsible for enabling and disabling the "Apply changes" button.
Change-Id: Ic4755b13f30fb738a7cb1eebaddef0435ea61d34
Currently, the insert template dialog includes a back button in the
upper corner. Confirmation of abandoning unsaved changes was
accomplished in an overlay panel. This patch rewrites as a dialog
and updates the on-screen text.
Bug: T297792
Change-Id: Ifa2ff97c9284609ee2a784f455789c56a762ba50
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
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
These pieces are only relevant when the new "inlineDescription"
feature is enabled. In other words: This can't have an effect on
the old dialog.
The 2.5em left and right are from an old styling when the
parameter pages have been indented.
Change-Id: I022b0dd94ee66f7de114c055c3f453317a7f6131
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
onReplacePart is being called when templates are being moved up or down.
To prevent that the multipart message gets moved around passively e.g.
by one element being added above it, this patch adds it to the top after each movement.
Bug: T292829
Change-Id: I946c9bc4ba5e1d261aefbb28a8c642bb58964842
We forgot change the way that message is build while adding the link
in 07f105fd7. Now it gets parsed correctly and the link will open in
a new tab.
Bug: T284985
Change-Id: I1ed9dfdafd08d08c5aff45f4b74c540b35ec14a1
This is only relevant with the old design. It's only noticable when
a field shows all 3 action icons: info, raw wikitext mode, and
trashcan. The last icon can wrap to the next line when the screen
is very narrow.
I tried to apply nowrap, but this causes other style issues.
Removing the arbitrary width allows the action container to be
as wide as it needs to be. I can't tell why this restriction was
there. It is in no way necessary, as far as I can see. I can only
guess it's a temporary artifact from when the dialog was designed.
Bug: T296730
Change-Id: I77129ccc3afe002ba697b1787b41d0a388d5f4b8
This does have a significant impact on the performance of the
template dialog. Not only on construction time, but also because
MWExpandableContentElement objects do some quite expensive
.updateSize() calculations the moment they become visible.
I profiled a template with (only) 200 undocumented parameters.
Construction time goes down from ~600ms to ~520ms. The mentioned
.updateSize() runtime goes down from ~300ms to ~10ms.
Bug: T296335
Change-Id: I280f814e722b299aae0ec6a5a2fa59292e3e5887
This doesn't have much of an impact on performance according to my
profiling. But I think it's worth it nevertheless. The idea is to
skip that <div> entirely when it's empty.
Bug: T296335
Change-Id: Id155725fbc2e3453acc1cdcabfdc2d687285d694
In OOUI the close button is always on the left side. See
https://doc.wikimedia.org/oojs-ui/master/demos/?page=dialogs&theme=wikimediaui&direction=ltr&platform=desktop
The CSS hack to move it to the other side doesn't work and must be
removed. You can see the problem the moment the text is longer (which
can easily happen in translated versions).
I tried to come up with a more official way to move the button to the
other side, but gave up. One way is to replace the existing
flags: [ 'safe', 'close' ]
with:
flags: [ 'primary', 'close' ],
framed: false
But this causes other style problems. Let's remove the bogus CSS
first and possibly try again in a later patch.
Bug: T294839
Change-Id: Ia6ddefd99e4a03a87b0450ab94712ff19bb268e4
There was a remaining issue when the window was made very narrow in
desktop mode (smaller than 500px). This patch doesn't aim to really
"fix" the dialog's design in this case. The goal is to make the popup
window appear less broken, so the text can stil be read and the
buttons clicked. That's all.
This patch should not have any effect in:
a) mobile mode,
b) desktop mode when the window is wider than 500px.
Bug: T294839
Change-Id: I3171dbb991533b91eaadba63b78d0ff40aa486dc
Same as I980b72c, but with another shortcut. Again. We tried
Ctrl+Shift+X before (conflicts with RTL feature in VisualEditor), as
well as Ctrl+Alt+D (conflicts with "hide all windows" incUbuntu).
Bug: T294905
Change-Id: Iae7ba759fcd7c107ef586bd5d9ae3cdbe445cedc
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
Screen readers will read the collapsed content despite the fact that
it is not visible due to the way it is hidden with CSS. The more button
makes no sense for these users and is probably more confusing than
helpfull.
Bug: T291277
Change-Id: I71888d8b9565d5ee85c5e7a48965e9e9a76eb984
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
For example, the Citoid extension adds a "change reference type"
button which should be visible when editing a citation template.
TODO: Decide whether we want to hide the "hide options" button in
this case. This should be handled in a separate patch and possibly
in a follow-up phase, it requires deeper changes to the logic.
Bug: T294351
Change-Id: I1c6c322fe48044d7e726bf20ba7cd2eda422cd8b
I can't reproduce this error, but I feel better having this safe
guard in place. In theory it's possible to trigger resize events
before the TransclusionModel is initialized.
Change-Id: I4bbac0f73873813629ff854ee728465c6e2a4ba7
I6909b3f0b2c153b7ee9995441e995ffa793eab40 was rebased, but
I0226ca7d39e04a69617c0d8a5d3c293cfc9e0709 was merged in the meantime.
Bug: T293202
Change-Id: I3cfec9ebc135eaf998a0982f458cfe75bfb2f01c
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
This is done for a lot of the elements in this class. They are trivial
jQuery elements instead of OOUI widgets. While we usually want to use
OOUI widgets, this is different in this case. Think of a template with
1000 parameters.
Bug: T291284
Change-Id: Ie1960ee706dca17aa4963c23a2e89c1cfff106f9
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
Preserve the place of annotation meta tags; adds information for the
users about annotation and, if necessary, annotation range extension.
The messages and individual handling of annotations for the annotation
range can be defined by the extensions: see I0b58a418 for an example
of how that can look like.
The structure of this patch closely follows the one from I104e7abbd
(handling of <noinclude> et al.).
Bug: T261181
Change-Id: I39029e4a63d22b37107edec066006557bcff34bf
Removing the selected item causes StackLayout to select (and scroll
to) its first item. To prevent this, we preemptively unselect.
Note that even when an unselected item is removed, StackLayout still
clears the selection, so this patch doesn't lose any useful
behaviors.
Fix should be pushed down into OOUI, unless there's a use case where
we want to select the first item?
Bug: T293635
Change-Id: I0c1fddfa32b76621a9f1328c8173f0158386aee8
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
This patch fixes two issues:
* The bottom corner of the new sidebar was never correct because of
a `padding: 2px` that was introduced later, but never compensated
for.
* The moment the toolbar is shown it's not a single-template dialog
any more. This implies minor style changes.
Bug: T290262
Bug: T292727
Change-Id: I08da73880c469085994ee4beb3fcdd973f80ae11
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 includes some moving of code. These helper functions seem to make
more sense in the TemplateDialog class.
Bug: T292371
Change-Id: I004405bab60a569b084f9083fefa41f44f9a5561
I hope this code is a little cleaner. This patch doesn't change how
the dialog behaves.
Bug: T292210
Change-Id: I237812b3404437948eb76d8b36dcca2a4c688d6d
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
It's not only used as an event handler, but called as an ordinary
method as well. Let the name reflect this better.
Change-Id: Ie5a0d9c4cd072063a164886f18d0859327b3f267
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
Reasserts scroll and highlighting when toggling the sidebar, in the
case that the other panel is hidden (narrow-view mode).
FIXME: Doesn't reassert focus because the page.focus() and
bookletLayout.focus() methods don't seem to work, maybe the bug is
especially prominent when the item to focus was already marked
active.
FIXME: Stopped working for right-to-left sync on wikitext elements.
This is a less common use case and can be addressed in follow-up.
Bug: T290975
Change-Id: I94f5709e810c63ee5fd7729a192ac7b92686b88f
… 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
