Commit graph

62 commits

Author SHA1 Message Date
Thiemo Kreuz 5eb1193670 Tweaks to focus/scrolling code relative to sticky header
* Rename method because it turns out it is not only about the sticky
  header, but also relevant when there is no header.
* Move some code to more appropriate places.
* Use 0 as documented in OOUI, not null.
* Set the padding back to 0 when the sticky header is not visible.
  As of now this is an unreachable state because the filters never go
  away after they have been made visible. Still this code was always
  written with this possibility in mind to make it future-proof.
* Performance optimization for the boolean "show filters?" check.

Bug: T312926
Change-Id: Iaba08ccd8bf00360fd26f9268d5be43df4f4fbd8
2022-08-02 11:41:23 +00:00
Thiemo Kreuz 0b28414dff Restore scrolling parameters into view with(out) sticky header
This is a partial revert of Ide45141. Now the scrolling always
happens (again), but properly considers the presence of the sticky
header. It was also not correctly initialized on construction time.

This is a candidate for a backport. The patch is intentionally as
small as possible because of this. Code cleanup will be done in a
later patch.

Bug: T312926
Change-Id: I06425b42566bfb2087846636055ee75e98a05029
2022-08-02 12:29:06 +02:00
Thiemo Kreuz 800461efac Scroll below sticky header only if sticky header is present
Bug: T312926
Change-Id: Ide45141f0a21b782f8674ecbed9ee512de985661
2022-08-01 15:37:33 +02:00
jenkins-bot a1bd30f35d Merge "Simplify partly redundant findFirstSelectedItem implementation" 2022-08-01 11:45:08 +00:00
Thiemo Kreuz d01d1e78c2 Simplify partly redundant findFirstSelectedItem implementation
Introduced via Ibc56abf. But the OOUI SelectWidget does have some
methods for this already.

This patch also updates some @mixins documentation.

Bug: T302965
Change-Id: Iffbb44d41586786a2165f8d7916f94a52ad19122
2022-08-01 11:07:29 +00:00
Thiemo Kreuz cdb9766752 Fix forgotten "setPage" property because of merge conflict
The rename in I5a16ab4 was done the exact same time a new usage was
introduced via Ibb717ca. Neither patch shows a conflict. Still there
is one.

I remove the extra `|| null` line because it is just not needed. We
don't need to set the property to null when it was already null.

Bug: T312213
Change-Id: Id3f025786c9412e8c1946434113c41356da08098
2022-08-01 09:42:25 +02:00
Thiemo Kreuz 19edc6043a Tab to first template parameter in list, not to the selection
This is a partial revert of Iaf089f4. It restores the old behavior:
* In case there is already a highlight in the parameter list, just
  keep that. Usually there is no highlight at this point, but better
  have this check in place to be sure.
* Otherwise always start at the top.

Jumping to the selection is confusing, esp. for keyboard-only users.
The argument goes like this:
* Let's say I'm in the middle of editing values on the right side of
  the dialog.
* I want to navigate to the sidebar. How do I do this with the
  keyboard? I use the tab key.
* Pressing tab also implies I move the selection to the next
  parameter. And the next. Until I reach the end of the parameter
  list. Then the selection stays there.
* When I finally reach the sidebar and tab into the parameter list,
  the last parameter is selected. But this was merely a side-effect
  of me navigating the dialog.

Such a "selection becomes highlighting" behavior was not specified
in T311204.

This patch is requested and approved by PM.

Bug: T312647
Bug: T311204
Change-Id: Ie5b5dfd4fca132050815e6182845ca23adb5f805
2022-07-21 10:44:48 +02:00
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