The main motivation is to get rid of the vague method name
"setParameter" that was previously used for three different methods
in three different classes. Now the three methods have three
different names.
Change-Id: I938de30b368daf6ce3385b2ed2bca98f316593e1
We would love to name this state "selected", but that term is already
used for a template parameter that is checked/used. The idea of "set"
was to have a list of parameters where one is "set". But the word is
confusing. I suggest "active page" because the entire purpose of the
blue selection is to highlight the currently active page (i.e. the
one you currently interact with on the right side of the dialog) in
the sidebar.
Change-Id: I5a16ab4c193ea05c21bb3bf89ada2ef550d8d6bc
I hope this makes it a little more readable. The two steps done in
the loop are mostly independent:
a) Find pages that should be removed.
b) Find next best top-level page when the current one is removed.
Change-Id: I600253fb206a31ef5851865e733b66c336d5014d
This does not have much of an effect, but can cause visual glitches
in rare situations. One goes like this: Use the keyboard and tab key
to navigate to a list of parameters in the template dialog. Press
space to enable the checkbox. The parameter gets a blue background
(= it's now the active a.k.a. "set" item). Press space again. Blue
disappears, as it should. Press space again. Blue is now missing.
Bug: T312213
Change-Id: I3071ec4d0a05e3505ec5216acc5a97b8eaf6f5d5
1. Before, removePages() was calling setPage() with null. This makes
sense for removed top-level parts because these are really removed
from both sides of the dialog. Template parameters are never removed.
Only unchecked (and removed from the right side of the dialog, but
this is not what this code is about). When I navigate a parameter
list and uncheck a parameter I need the focus and highlighting to
stay.
2. We have a dedicated method when a parameter is unchecked. This
can check if the removed parameter is also the selected one (called
"set" in this code) and can reset this state. Without losing the
highlighting or anything else.
Bug: T312213
Change-Id: Ibb717ca49cae805617ebee196937c79daa72f1c1
There are many errors that are temporary in some way, and treating
them as unrecoverable is a poor experience.
Even for errors that really are unrecoverable, our interface works
poorly, because you need to hide the error message first to do
anything else, and you need to close the dialog to see it again.
This distinction is not really helpful, let's get rid of it.
Bug: T307330
Change-Id: I9680cc416da5b27881aeb3502f506dcb5d4bb71f
Optimization: Don't search for a checkbox that represents a not yet
named parameter placeholder. There is never one.
Fix: Store null, not undefined.
Bug: T312213
Change-Id: I395008f15d13133ad456d0a77571b7aa1c7a7fc9
Steps to reproduce:
* Set a breakpoint or debug.log() at the start of
TransclusionOutlineWidget.setSelectionByPageName()
* Edit a multi-part template.
* Use the keyboard to navigate to a template name in the sidebar.
* Press space.
This is currently triggered twice. Let's get rid of the more obscure
one. It was introduced as part of Ic4ee673. I don't really know why.
Bug: T313207
Change-Id: I3ddc072f5d42c17249abc82026e0bf1a4be1dc6e
This also means we have to move the declaration/documentation of the
event up one level into the generic "part" widget.
Change-Id: I1b803201f8955b58136ee7f37c04c01edcd47395
The code that receives this event does not need to know that the
source is a "parameter". It's just some "item" in the sidebar. The
idea is to reuse the event for both top-level parts as well as
parameters. This will be done in the next patch.
Change-Id: I858040f5adf8e156b6013caaa527b3237b7bac0f
What I found is a single `$container.animate( animations )` that's
responsible for the misbehavior described in T312768.
This method is called from a few places. I think none of these places
benefits from making this an animation. This is often more distracting
than helpful, especially when navigating with the keyboard.
Bug: T312768
Change-Id: I90a80d6ae8c1b47ee22297d2520255cad890b90e
Fix setParameter() and let it (un)highlight parameters independent
from a selection (called "set" here).
Bug: T312925
Change-Id: Ie4e9ba94659f4f70160193ca6bec804f8a4473e4
These comments just repeat what the code says. Most of them are
copied from OOUI. Some talk about BookletLayout – a class name we
don't use any more.
Change-Id: I7ce354dfe059657395103ffef767eb8f6d37bfb7
New changes:
14ab5f36c Localisation updates from https://translatewiki.net.
ffde59c5f build: Replace automatic 'grunt-svgmin' with manual 'svgo' script
6294cc572 Re-crush SVGs now we're on SVGO 2.x not 1.x
42e35e898 Visual diff: Use mix-blend-mode to improve opacity hacks for content in highlighted areas
Bug: T311903
Bug: T312013
Change-Id: I0428ca6038459087d6c6f79e20ffeaa32659c3db
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
The optimization removed in this patch is done twice. The effect is
that the loop does not end when it finds a match but the match happens
to be the current page.
This doesn't change anything. It's really only about performance.
Change-Id: I5c2de101eb2f14f814f00cf7eacf46b70346f4c8
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 line was added as part of I0229b63. While what the comment says
is still true to some degree, the line stopped being helpful when
we introduced sticky headers. When scrolling the sidebar for a
multi-part template with many parameters the sticky header jumps up
and down by this 1 pixel.
TL;DR: This is one of these hacks where it's better to remove it and
look for a better solution when we notice the original issue again.
Bug: T312768
Change-Id: I2fedea4e1d4d6c95c74a63c522821a6ebc2ee2b2
250ms gaps are pretty common when typing at a normal speed, resulting
in a lot of (often expensive) API requests (e.g. T312319). For most
use cases this level of responsiveness in the preview is not necessary.
Change-Id: I6504567d4da02a66171ee35a9c4fd35c85136909
This patch contains a small cleanup that moves the scrollIntoView into onSidebarItemSelected.
Since all other places already have a focus call, scrollIntoView is not needed there.
Bug: T312850
Change-Id: I1a3cb3905faea2bd8a6bf8dc4cfd748813e1c875
