Commit graph

163 commits

Author SHA1 Message Date
Thiemo Kreuz ffb7d76263 Move .containsValuableData() method from dialog to model
… where it belongs.

Change-Id: I522b888e366f066b28983a18041a8728d11623df
2021-09-27 11:16:54 +02:00
Adam Wight 02c637ad16 Sync panels in narrow-view mode
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
2021-09-27 10:12:36 +02:00
jenkins-bot 78c1f282c7 Merge "Always update mode on replace part" 2021-09-22 10:14:31 +00:00
WMDE-Fisch 129551d869 Always update mode on replace part
AddPart in the resetDialog also triggers replace part
so this call could be removed there.

Bug: T291365
Change-Id: Id6c0f5bf3aaece45da37ffab75a4d99c113944f6
2021-09-22 11:30:52 +02:00
Thiemo Kreuz e037f99930 Merge custom "focusin" with "set" event handler
This code was introduced in I8fafee6. I can't tell any more what
the "bug" mentioned in the commit message was. Let's get rid of the
duplicate code path, see if we run into regressions and deal with
them one by one. That's much easier to handle than keeping this
confusing code path around.

Note this "focusin" event handler was actually re-implementing
parts of the upstream BookletLayout, namely
OO.ui.BookletLayout.onStackLayoutFocus().

Bug: T289043
Bug: T291381
Change-Id: Ib386ae6efec08465122f0e8ee81cd6dc9a2d337a
2021-09-21 10:14:57 +00:00
Thiemo Kreuz 48b9a58737 Regression: Update selection in new sidebar relative to content
We broke something with the change I166b971. When we renamed that
method it started to override (and therefor disable) the method
with the same name from the base class.

I decided to move all code in the subclass for the moment. It might
be misplaced there (note how almost all code related to the new
sidebar is in the base class). But this is cleanup work for later.

Bug: T289043
Bug: T291151
Change-Id: Id255585e78967eee0f72c27727cd23211674923c
2021-09-17 16:45:24 +02:00
jenkins-bot c24e6a85d9 Merge "Rename weird .onBookletLayoutSet() event handler" 2021-09-16 12:39:55 +00:00
jenkins-bot 3486d9ea58 Merge "Rename showSidebar to expandSidebar for consistency" 2021-09-16 12:36:09 +00:00
Thiemo Kreuz 0fcfa5c4aa Rename weird .onBookletLayoutSet() event handler
It was called like this because the event is literally called "set".
But it doesn't explain _what_ is set.

The new name .onBookletLayoutSetPage() already appears somewhere
else in the codebase.

Change-Id: I166b971c08f5d0fae97fc9d6244117a680f84b7c
2021-09-16 14:18:57 +02:00
Thiemo Kreuz cf14790450 Utilize native .isRemovable() in template dialog sidebar
The OO.ui.OutlineOptionWidget class does have an .isRemovable()
state. This is how OO.ui.OutlineControlsWidget decides if the
remove button can be used.

It appears like ve.ui.MWParameterPage forgot to mark required
parameters as not removable.

This makes some oddly specific code in ve.ui.MWTransclusionDialog
obsolete. Note how this class does not contain any other code
about "required" or other flags specific to parameters.

Note that one aspect of this patch will be visible in the old
sidebar: The trashcan will be visible on required parameters, but
disabled. It was hidden before. However, this actually improves
the UX. Hiding the trashcan made the up/down buttons jump around.
This makes it unnecessary hard to hit them. It also causes visual
distraction when navigating the list of parameters.

Let's stick to the upstream OOUI behavior.

The remove button still disappears when the only element in the
dialog is the template search widget. This is clearly an
entirely different state. (Don't ask why the up/down buttons are
not hidden. It was like this before.)

Bug: T289039
Change-Id: If78881e503f19f497f1993da4e5b9b09ee538307
2021-09-16 10:03:09 +02:00
Thiemo Kreuz 6680f5f141 Force delete button to be disabled on template parameters
It's impossible to use the delete button from the
OO.ui.OutlineControlsWidget to delete a parameter when the new
sidebar is active.

This partly fixes one of the issues mentioned in I97d77f4. The
delete button might stil become active for a moment (not fixed
yet), but doesn't stay active forever any more with this fix
in place.

Apparently this also makes another workaround obsolete.

Change-Id: I0bca310772c26149170af23ff8e5505c3ce4adf4
2021-09-13 16:01:25 +02:00
jenkins-bot 9ef1b22b37 Merge "Clean up post- OOUI fix" 2021-09-10 14:37:12 +00:00
Adam Wight f8cf9e9b8e Clean up post- OOUI fix
Pure cleanup, doesn't change behavior.

Change If8da5ae85dff63c34 included in OOUI v0.42.0 tracks invisible
controls, so it's no longer necessary to maintain persistent class
variables pointing to the buttons.

Also simplify repeated logic to make it clearly exclusive.

Bug: T290554
Change-Id: If9b6404d7061999540515645fa8e50b9a21f5a21
2021-09-10 09:39:07 +00:00
Andrew Kostka 97ef584e20 Rename showSidebar to expandSidebar for consistency
Bug: T274554
Change-Id: Ia0611fe10492e05e4625eb743c66eac7d0235dc5
2021-09-09 12:26:40 +02:00
Andrew Kostka 0c14b6d0bb Don't jump back-and-forth between fullscreen and floating when resizing
Previously, when the tranclusion dialog was being resized, there was
a specific width at which both the sidebar was collapsed and the dialog
was downsized from "larger/large" to "medium". This resulted in the
dialog switching from fullscreen to floating, since the dialog's width
breakpoint for "medium" doesn't match our width breakpoint for mobile.
If the user continued to downsize past this breakpoint, then the dialog
would eventually switch back to fullscreen resulting in this weird
behavior.

A simple way to prevent this is to avoid changing the dialog's size
at all. Since we don't support collapsing the new sidebar (unless
already in fullscreen), we can just leave the dialog at the
"larger/large" size.

Bug: T274554
Change-Id: I5460cdfb1a7ed73fe7957745ba37055c5f66dce1
2021-09-09 12:16:52 +02:00
Thiemo Kreuz 0ba7480901 More complete top-level part selection support in new sidebar
This comes with a few significant changes:
* A whole bunch of places in the code that focus and highlight
  an element in the old sidebar consider the new sidebar now.
* Same when e.g. the toolbar at the bottom needs to know which
  part is selected. This is read from the new sidebar now.
* To make this possible I had to merge the small helper class
  we introduced in I7bc73cc back into the dialog.

It's helpful to understand how the event flow works:
* You click a template name. This does nothing (does not select
  the element). It only triggers an event.
* The event is catched by the outer container that manages
  all parts. From there all elements are unselected, and one
  selected. This call is internal and should not trigger
  another event.

Bug: T285323
Bug: T288827
Bug: T289043
Change-Id: I4a2d2b83cf2691423d4b0e6f4487228fa3c7b56d
2021-08-27 19:16:00 +02:00
jenkins-bot 6c33cc4a35 Merge "Add message on multipart content" 2021-08-26 07:34:15 +00:00
WMDE-Fisch 67988452ab Add message on multipart content
Bug: T276574
Change-Id: Iff5c2fd4071dd03ca62c395cce4041dacb1c7ea6
2021-08-25 10:47:16 +02:00
Adam Wight b1a376a6e1 Clean up toggleSidebar by splitting out 'auto'
This naturally fits into a new function.

Bug: T274554
Change-Id: Icda432dc1a13243efddc9949f41a00162053970d
2021-08-23 14:38:24 +02:00
WMDE-Fisch 53186fc231 Remove sidebar margin on single transclusion
Bug: T272482
Change-Id: I5696dbd469161b1ab1ede2bb3f49e6dd24b98521
2021-08-17 13:54:36 +02:00
Andrew Kostka a2135d524d Show sidebar controls only when multi-part content is present
Bug: T272482
Change-Id: I092438bc9c7d5569f758786ca6952beffdf37507
2021-08-11 15:00:59 +02:00
jenkins-bot aa868059ee Merge "Better name for ambiguous "empty" concept in the model" 2021-08-09 09:40:40 +00:00
Thiemo Kreuz 35080a8646 Better name for ambiguous "empty" concept in the model
This is really only about the methods name, but doesn't change
any behavior.

I realized we work with several different definitions of what
"empty" means. There are at least two significant definitions:

1. When a parameter's value is the empty string or identical
to the default value, the behavior of the template is the same.
It will use the default value just as if the user entered it.

The auto-value is a meaningful value in this scenario and can't
be considered equal to the empty string.

2. The context here is when the user presses the back button.
This will destroy all user input. But an auto-value is not user
input. It will appear again when the user realizes they made a
mistake. Nothing is lost.

Personally, I would not use the word "empty" to describe this
concept. Things like "containsUserProvidedValue",
"isCustomValue", "isMeaningfulValue", … come to mind. These are
all still a big vague. A "user provided" value can be identical
to the default or auto-value. "Custom" how? I went for
"containsValuableData" for now.

Bug: T274551
Change-Id: I2912a35556795c867a6b2396cbad291e947f0ed6
2021-08-06 15:11:44 +02:00
Andrew Kostka 21e9f5f0c4 Make the sidebar in the transclusion dialog responsive
Bug: T274554
Change-Id: I51e988e6fb0e50c9d542400471152807626539ea
2021-08-05 17:23:22 +02:00
Andrew Kostka 2f1cfdd187 Cleanup feature flags in ve.ui.MWTransclusionDialog
Change-Id: I6f6326c981da998b547a2a1990e26185763093ca
2021-08-05 17:00:27 +02:00
jenkins-bot cf17a1f191 Merge "Remove tooltip from template dialog title" 2021-08-03 15:29:10 +00:00
jenkins-bot e57daaad98 Merge "Minor JSDoc documentation fixes in template dialog code" 2021-07-29 12:13:17 +00:00
Thiemo Kreuz 014bc9369c Fix incomplete back button implementation in template dialog
Bug: T286783
Change-Id: Ie86e2c8c6a874fde11591286a83faba71a34430d
2021-07-16 14:54:09 +02:00
Thiemo Kreuz 092276009a Minor JSDoc documentation fixes in template dialog code
Change-Id: I3697e48697ab0910b90b418abbc370736589b6e1
2021-07-16 14:25:11 +02:00
Thiemo Kreuz 55a49195ba Add @private/@protected documentation to template dialog code
These tags don't do much, if anything. But they provide a hint
in which scope a method might be used.

Bug: T284895
Change-Id: I0b4bdd416ee89d26961c4ded4d8bbace8c57da76
2021-07-01 09:36:00 +02:00
Thiemo Kreuz 4367235dcc Inline many var declarations in the code below
This makes the code more readable and easier to reason about.
The ESLint rule responsible for this code style was removed
just recently.

Notes:
* I focus on classes that are relevant for what the WMDE team
  does right now.
* I merge multiple `var` keywords only when the variables are
  strongly connected.
* Caching the length in a for loop makes the code hard to
  read, but not really faster when it's a trivial property
  access anyway.

Bug: T284895
Change-Id: I621fed61d894a83dc95f58129bbe679d82b0f5f5
2021-06-23 09:02:24 +00:00
Thiemo Kreuz eedef37c78 Remove tooltip from template dialog title
This reverts parts of I678bb24.

Brief history of this code:
2014: The dialog was designed to dynamically change the title.
      There was never a tooltip.
2016: A change in OOjs changed the behavior in VE. Now the initial
      title shows up as a tooltip. It never updates because VE
      does not know about this. The tooltip does not match the
      visible title.
2021: We revert to the behavior from 2014. We achieve this by
      bypassing the codepath that creates the tooltip. This is why
      ….title.setLabel() is used instead of ….static.title.

Bug: T276568
Change-Id: I346a904881c3a63186d6a80afdaf717688bab42a
2021-06-22 16:12:52 +02:00
Adam Wight 1100db9665 Revert "Make template dialog header tooltip match title"
This reverts commit 7d05257059.

Change-Id: I16395c1118a5a1afe6c0f9022e478f38a2ac3755
2021-06-22 15:37:09 +02:00
Adam Wight 7d05257059 Make template dialog header tooltip match title
The tooltip is useful for languages where the dialog title might get
truncated.  This patch makes sure the tooltip is always the same as
the visible label.

Bug: T276568
Change-Id: I678bb243bb5ac6d1c516ee4e146f2db9ffd5afcf
2021-06-22 13:16:17 +02:00
Thiemo Kreuz c6f2683dd8 Rewrite MWTransclusionDialog methods for readability
Most notably:
* Introduce variable names that explain much better what's
  going on.
* Reduce nesting.

Bug: T284895
Change-Id: I793677d8107abb6354f9e19d79c4879a41c4bd93
2021-06-18 16:09:52 +02:00
Thiemo Kreuz bba592460d Better variable names for the template dialog's "single" mode
The "mode" button is the button that allows to expand and
collapse the dialog. It can't be collapsed when multiple
templates are edited. That's what these lines do,
disabling the button.

"Can expand" is not the correct question. It's always
possible to expand the dialog no matter what it contains.

Bug: T284895
Change-Id: I60f3060695c80bf5541ef2156be89b85a62bf91b
2021-06-17 18:24:17 +02:00
jenkins-bot 6d101fb9d7 Merge "Extract "show all" to placeholder class" 2021-06-16 12:25:55 +00:00
jenkins-bot 09933c4905 Merge "Method for getting the currently selected element" 2021-06-16 07:22:28 +00:00
jenkins-bot 4c7f5ff805 Merge "Introduce a new class to encapsulate booklet inter-pane wiring" 2021-06-15 14:55:16 +00:00
Adam Wight 950a5300dc Extract "show all" to placeholder class
We can skip all the up and down message passing by persisting the
parameter placeholders for each template dialog.  If the parameter
list is expanded then the placeholder is deleted, on being created
again it will still have state.

To test: create a transclusion with two templates, each having many
parameters.  "Add more information" to add parameters, expand the
list by clicking "Show <num> more fields", then delete the parameter
placeholder using the trash cans.  Try different permutations to fool
the cache or collide with another template.

This is preparation for other template sidebar dialog work.

Bug: T284636
Change-Id: I23bdd38b173114c2a9afafc7465c4beb92d25869
2021-06-14 18:09:26 +02:00
jenkins-bot 3f4cfb0fd8 Merge "Rename "value" to "wikitext" in TransclusionContent… classes" 2021-06-11 14:10:24 +00:00
WMDE-Fisch 09d98669df Rename expand and collapse button msg key names
These are currently more confusing than helpfull. Also applied
alphabetical order.

Change-Id: I4d48f7bc28b403142a5818d1c7a062872afc660d
2021-06-11 13:16:44 +02:00
Adam Wight a2daf5f6c6 Method for getting the currently selected element
Continue refactoring booklet pane access.

Bug: T284632
Change-Id: I866f8550d269d05b40eaddf8abb4ed5be03f902d
2021-06-10 15:32:07 +02:00
Adam Wight 44035a2384 Introduce a new class to encapsulate booklet inter-pane wiring
Begin to extract the wiring between a sidebar and the content pane of
the template dialog booklet layout.  Eventually, this helper class
will present a high-level interface like "addPart(id)" and will take
care of creating the outline item, content page, and connecting
events.

Start very simple, take over the "focus" method.

Bug: T284632
Change-Id: I7bc73cc4386b99d95941fc6ed88ab5bd998de014
2021-06-10 15:30:51 +02:00
Thiemo Kreuz df419335bb Rename MWTransclusionDialog.setMode() to toggleSidebar()
This reflects better what the method actually does. This patch
is a direct follow-up for the renames started in Ib029fd4.

Change-Id: Ie3e87139a5c2f5ac196e0fcc02fb897fadc99177
2021-06-10 14:05:40 +02:00
WMDE-Fisch ecf317e0bd Fix message keys to show options in template dialog
The names of the messages keys are very confusing. The order was set
wrong during refactoring in Ib029fd48b393d2ab7d7cff6c842789e22989e944.

We should rename the keys in a follow up in sync with translatewiki.

Bug: T284649
Change-Id: I43794d80b7df7d00441cb583ca53bcab03999e65
2021-06-10 10:50:40 +00:00
Thiemo Kreuz 190237ee28 Rename sidebar state to distinguish from multiple transclusion
This dramatically simplifies the "mode" flag in
MWTransclusionDialog. The main reason to touch this code is:
The flag appears like it will be "single" when the dialog
contains a single template, and "multiple" when there are
multiple templates. But this is not true.

What the flag really does is show/hide the sidebar. The sidebar
is needed to be able to create multi-part templates. But a
dialog that already contains multiple templates can be set to
"single" mode (i.e. the user can collapse the sidebar), and
vice versa.

This patch focuses on private details inside of this class, but
keeps the terminology of a "mode" in some places. E.g. the
messages are not renamed to not cause unnecessary trouble for
translators.

Change-Id: Ib029fd48b393d2ab7d7cff6c842789e22989e944
2021-06-09 09:44:08 +02:00
jenkins-bot 2b0da3e338 Merge "Update template dialog title when multiple transclusions present" 2021-06-08 08:14:11 +00:00
Thiemo Kreuz 8d7c55aa71 Rename "value" to "wikitext" in TransclusionContent… classes
This class represents a raw wikitext snippet. There is also no
base class that would require us to follow a generic
getValue/setValue naming scheme.

Change-Id: I0891a2f6c0ae0121429a47c39221e99b9653e8e3
2021-06-08 09:27:47 +02:00
Andrew Kostka 67e48a26ef Update template dialog title when multiple transclusions present
Bug: T276568
Change-Id: I530a6cd8aea4f9fcafe7a03fc5050883f1f4b37c
2021-06-04 16:48:05 +02:00