Commit graph

55 commits

Author SHA1 Message Date
jenkins-bot 18dade9ffa Merge "Rename confusing "itemSet" to "active parameter"" 2022-07-20 10:05:01 +00:00
jenkins-bot 4a3d83d087 Merge "Rename confusing "isSet" to "active page indicator"" 2022-07-20 10:04:58 +00:00
Thiemo Kreuz 7a59c4696a Rename confusing "itemSet" to "active parameter"
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
2022-07-20 11:42:42 +02:00
Thiemo Kreuz 4782987a7f Rename confusing "isSet" to "active page indicator"
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
2022-07-20 09:38:41 +00:00
Thiemo Kreuz 74bffc5d0d Fix sidebar losing focus when unchecking params with space
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
2022-07-20 09:19:59 +00:00
Thiemo Kreuz e2cce0dd3d Minor fixes and optimizations to ParameterSelectWidget
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
2022-07-19 16:36:30 +02:00
jenkins-bot 684a69c17e Merge "Unhighlight parameter on blur" 2022-07-18 11:47:08 +00:00
Thiemo Kreuz 569ad3d538 Remove/fix obsolete code trying to un-highlight parameters
Fix setParameter() and let it (un)highlight parameters independent
from a selection (called "set" here).

Bug: T312925
Change-Id: Ie4e9ba94659f4f70160193ca6bec804f8a4473e4
2022-07-18 13:04:42 +02:00
Adam Wight 42cbde12a8 Unhighlight parameter on blur
Bug: T312925
Change-Id: I5de5dbb5e0f9f2ae1eb8e5f2c07317cda50680ea
2022-07-18 12:59:23 +02:00
Thiemo Kreuz d57210c1c0 Rename misleading templateParameterSelectionChanged event
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
2022-07-15 14:19:19 +02:00
jenkins-bot b4b6940c64 Merge "Minor fixes to template dialog related code documentation" 2022-07-14 07:01:11 +00:00
WMDE-Fisch a4aa684308 Don't call parent on SelectWidget focus event
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
2022-07-13 15:27:24 +02:00
Thiemo Kreuz 466a73e730 Minor fixes to template dialog related code documentation
Change-Id: I0b5e0ede45578db23ecd5817baf842eb3cc3dbf4
2022-07-12 20:10:02 +02:00
jenkins-bot 55165649c7 Merge "Document some classes touched in the sidebar rewrite" 2022-07-12 13:53:29 +00:00
WMDE-Fisch ec522938b8 Scroll parameter items into view when hidden behind sticky
Triggers scroll in all situations where highlighting changes.

Including keyboard navigation.

Bug: T312542
Change-Id: I849f64c5cefe0cac3fde6e848b23b7b0cfc489ce
2022-07-12 10:52:19 +00:00
Adam Wight d77728bd4f Document some classes touched in the sidebar rewrite
Depends-On: I385dca1d95033961d3844e888521750443e49c95
Change-Id: Ic515b41863bcf2ac4daa0272f67e1d30238a4b85
2022-07-12 12:49:31 +02:00
WMDE-Fisch b75082009a Differ between setting and highlighting a parameter
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
2022-07-12 10:18:56 +00:00
Adam Wight a83d37d864 Revert "Restore selection after keyboard parameter navigation"
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
2022-07-07 15:24:11 +02:00
Adam Wight 787d44af66 Restore selection after keyboard parameter navigation
When blurring out of keyboard parameter navigation, the
last-selected parameter will be highlighted again.

Bug: T312017
Change-Id: I8b0fb667b44b324529d4c45c39bf21573517f989
2022-07-05 13:37:38 +02:00
Adam Wight 6c16f93504 Prevent highlighting the first parameter when clicking to remove
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
2022-07-04 15:50:56 +02:00
Adam Wight aa7501a8e3 Select parameters like templates
Bug: T311204
Change-Id: I61294087963f4659f065f011a2cac350cfd35dda
2022-06-30 14:23:41 +02:00
WMDE-Fisch 96dc96da8d Extract template scrollToTop method
Also putting the findFirstSelected to where it belongs.

Bug: T302965
Change-Id: Ibc56abfdf84a0b5eade5b6a44903d769f424206d
2022-03-11 12:30:51 +01:00
Thiemo Kreuz 836d36bc16 Skip initial creation of unused parameter widget
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
2022-03-02 10:19:46 +00:00
Thiemo Kreuz bc0a34b0ed Add and fix default values in parameter documentation
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
2022-02-21 13:19:21 +00:00
WMDE-Fisch ce230afa09 Disable hide unused widget when all are selected
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
2022-02-09 09:23:14 +00:00
WMDE-Fisch 3e28977087 Remove local aria-multiselectable
Added to OOUI v0.42.1 see I717ff53e18e95c72ace1c46b6236cfecb5cd8217

Bug: T291284
Change-Id: I45965181f22f9c5845f6dd44ab75e95305a43dce
2021-11-05 09:10:21 +01:00
WMDE-Fisch f9fda0594f Introducing a mixin for ARIA descriptions on parameter selection
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
2021-11-01 13:20:00 +01:00
WMDE-Fisch b84d5f99d6 Set aria-multiselect on OutlineParameterSelectWidget
Bug: T291284
Change-Id: Iedb731d528b7f73c188aaa40eba74ba18dc8781e
2021-10-21 15:54:01 +02:00
Thiemo Kreuz 152f5edae2 Add/remove some TODO comments
Change-Id: I99bf367025ba4830fb683add93087d42dce4566d
2021-09-28 09:55:47 +02:00
Adam Wight bd7a1b0997 Unhighlight all items when highlighting
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
2021-09-24 09:26:56 +00:00
Adam Wight 7d9a61ded7 Document optional parameter
This function can also be used to unhighlight.

Change-Id: Ic95d75dfa862951613d7c3bc21a19ec967cf9805
2021-09-21 16:36:49 +02:00
jenkins-bot fe6c46bbdc Merge "Merge "templateParameterClick" and "choose" events" 2021-09-15 10:11:12 +00:00
Thiemo Kreuz c33536112d Avoid referencing methods via strings, take 2
The first approach in patch I024ac1e doesn't work.

Change-Id: I4c9f8ef65c9e423a3e34f48483142a97ec6cef1e
2021-09-14 12:40:59 +00:00
jenkins-bot 787bc0db17 Merge "Revert "Avoid referencing methods via strings"" 2021-09-13 18:55:22 +00:00
Bartosz Dziewoński a667045383 Revert "Avoid referencing methods via strings"
This reverts commit 48283d5f19.

Change-Id: Ifc2b7fb9e9fb7deaecb6c0475db0f9253e81869f
2021-09-13 18:36:07 +00:00
jenkins-bot a52cb05804 Merge "Avoid referencing methods via strings" 2021-09-13 13:28:19 +00:00
jenkins-bot 401319cb67 Merge "Split focus handling from add/remove parameter events" 2021-09-13 13:28:16 +00:00
jenkins-bot b4ac8c45ce Merge "Make "templateParameterClick" event similar to "choose"" 2021-09-13 13:24:37 +00:00
Thiemo Kreuz 48283d5f19 Avoid referencing methods via strings
Should be a bit more obvious, I hope.

Change-Id: I024ac1e2d68bfe4c5af733c48967af4c51f336c1
2021-09-13 14:26:16 +02:00
Thiemo Kreuz 338a800837 Merge "templateParameterClick" and "choose" events
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
2021-09-11 17:47:08 +00:00
Thiemo Kreuz 1493aa7a35 Split focus handling from add/remove parameter events
* 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
2021-09-11 17:47:02 +00:00
Thiemo Kreuz cdc168d32a Use OOUI 'classes' property instead of .addClass()
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
2021-09-11 17:41:45 +00:00
Thiemo Kreuz ec526ea64f Make "templateParameterClick" event similar to "choose"
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
2021-09-10 17:19:19 +02:00
Thiemo Kreuz 30d4096aeb Remove invisible tab target from template dialog sidebar
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
2021-09-10 06:42:22 +00:00
jenkins-bot 836ab09a79 Merge "Rename and document select/focus events in template dialog" 2021-09-03 09:38:21 +00:00
jenkins-bot 7848b1a071 Merge "Fix inconsistent keyboard :focus styles in template outline" 2021-09-02 13:04:17 +00:00
jenkins-bot 7c304dc444 Merge "Highlight first template parameter when using tab to navigate" 2021-09-02 10:18:22 +00:00
jenkins-bot c8e00fb98a Merge "Highlight corresponding template parameter in new sidebar" 2021-09-02 10:10:13 +00:00
Thiemo Kreuz 06cf00f4c6 Rename and document select/focus events in template dialog
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
2021-09-01 16:23:59 +00:00
Thiemo Kreuz fc0b419540 Highlight first template parameter when using tab to navigate
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
2021-09-01 14:05:01 +00:00