There was some confusion during the code review of
4dbe4076d6
In the case of array_merge, anything defined as the second parameter
with the same key as the first will override the data in the first. This
means the current situation is that parent data replaces any local data.
However, we want the local data to be preferred.
Specifically, the call to $this->getFooterData() must override
the data set parent::getTemplateData. This is not happening with the current
ordering.
Without this change, Ib2af1f72c0508870b20321dffc8af2158d0dbd55
will result in the footer not rendering.
Bug: T262730
Change-Id: Iaf701c054314f210e40a82b5277e9914edccc493
Kept as simple as possible for now. The new class is added but no classes
are removed. This will be done in a follow up.
Bug: T256897
Bug: T253938
Change-Id: Ib31a9d8f2ac14e63b63e82abd4a9aa1fcb956f45
The + operator will drop any existing keys. The data from
Vector should have precedence as the data coming from core may be
in a different format.
Noticed while upstreaming data for footer.
Change-Id: Icc772733577e0b165acdbc8a9b6143bd5410c33a
Following on from I0edbd89, set the data-search-loc attribute to
"header-navigation" for legacy Vector.
Bug: T256100
Change-Id: I113c56ac9b18df46ebc13b9985aea03ba82419bd
The anon placeholder is now provided by core. The styles will remain to
style it to suit Vector needs.
This ULS hack is no longer needed as the extension is already using array
plus operator which does set union, so it only appends the right-hand array
to the left-hand array. Some string concatenation that prevented it from
working correctly has now been resolved by Ice379cd in core.
Bug: T263382
Change-Id: I5691529ab8c59f4053cff38ea6f7dd01c326c074
With I5772eb7 merged in core, it's no longer accurate to say that the
$options param is optional.
It's important to note this now, before someone report it as a bug.
Since the method is a public API and technically does not require a
parameter, no one will expect that following the rules (providing no
argument, as in `$skin = new SkinVector`) will throw fatal exception.
Bug: T262233
Change-Id: I771e5cc8ff205943016a26854da18088817b9238
Given I7184f2f5dd7fdb49fb928265bbf711ef9fd555c9 we can now
use the data generated by core which has the benefit of being
configurable per language variant.
Bug: T261153
Change-Id: I7a77f164da53c362da225ae3d13635f9ddca529f
The mediawiki.searchSuggest protocol part of the SearchSatisfaction
instrumentation reads the value of an element with the "data-search-loc"
attribute and sets the event's inputLocation property accordingly.
Set the appropriate value for the attribute in
SkinVector::getTemplateData and render the attribute in
the SearchBox.mustache template.
Bug: T256100
Change-Id: I0edbd8906a042072b2055adc2889b083b2c6cac4
This will allow us to add the A/B testing requirement for logged in
users.
In preparation for the new A/B test requirement, a custom requirement
is added as the feature management system does not
support OR operations and the desired effect is the case where:
* the SearchInHeader feature flag has been enabled
* OR the SearchInHeaderABTest feature flag has been enabled and the user is bucketed
Bug: T259250
Change-Id: If948603bd598e1b5597345f4268736417f4c3a24
Rename buildSidebar to getTemplateDataSidebar to reflect what it is
doing in Vector rather than overloading it with template data generation.
Changing the return value has consequences on refactors
inside core a Skin::buildSidebar returns an array not an
associative array.
Change-Id: I0a4913c21af6ac3f5259cee8583aa121756251c1
This allows us easily to identify in templates what messages are
used and where, as well as allow us an easy way to tell when
messages are no longer being used.
Change-Id: I610224d551ebea54ae32e9e79901befe80cfd5ce
To support roll out and avoid issues with cached HTML the new
styles for the new search feature are restricted to HTML where
the body tag has `skin-vector-search-header` class.
For legacy mode, we introduce a new class
`skin-vector-search-header-legacy` and temporarily use a CSS3 `:not()`
selector to ensure the styles ship during the phase where cached
HTML can be served. While this will create some display issues in
browsers that do not support CSS3 selectors, all grade A browsers in
our compatability matrix support this.
Bug: T249363
Change-Id: I7f8059d43eaab49de362405784b34a4fe502c7b0
In PHP we add collapsible classes to all elements except watchstar
so that certain tabs can be collapsed under the more menu in JS.
This adds unnecessary complexity to our codebase and is not used
if JS is disabled.
To simplify this and bring Vector's PHP consistency with core this
logic is moved to JavaScript.
Bug: T259372
Change-Id: I2acbf7089198118626368ee8a37615d2de062f83
The has-logo property is redundant:
The Sidebar in legacy is its own component. The logo is always
rendered in the legacy sidebar.
data-emphasized-sidebar-action can be unconditionally sent
The legacy sidebar doesn't use it, so there is no need to conditionally
add a check for legacy
These two changes allow us to remove the isLegacy property in favor
of the single use of the isLegacy method.
Change-Id: Ie7ee3cec29b20d07db81799484627254d4a78f5f
Please note I7e06a4cc226f3434c0f655212a464b8b98bcc7f4 should be
merged at the same time as this patch.
== The background ==
All extensions have been weaned of BaseTemplate hooks in
Wikimedia projects.
This change now means that Vector will no longer run
any BaseTemplate hooks. See the epic T253809 for the
implementation details.
== The change ==
BaseTemplate will now have nothing to do with the rendering of
Vector. The skin version is added to express the significance of
breaking compatibility with 3rd party extensions.
We TEMPORARILY remove SkinVector to retain git blame. SkinTemplateVector will
be renamed SkinVector in the follow up (see 2/2)
Update skin.json to use SkinTemplateVector for the skin (this will be fixed
in a follow up).
The isLegacy method is moved to SkinTemplateVector.
Changes of note:
* html-debuglog is no longer needed. SkinMustache includes this information on
the skins behalf
* html-printtail and html-headelement are now not needed in the master template
and added by SkinMustache
* Skin::getAfterPortlet does not provide the `after-portlet` wrapping element provided
by BaseTemplate::getAfterPortlet so this is added
* SkinTemplate::getFooterIcons does not support the options that BaseTemplate::getFooterIcons
does so any icons which do not have an image must be manually checked for and unset
Known changes to HTML output as a result of intentionally
delegating their output to the core SkinMustache class:
* A new line is removed between the body element and #mw-page-base
* #mw-html-debug-log now appears at the end of the body element
* #printfooter is now a child of #mw-content-text rather than sibling.
Bug: T251212
Change-Id: I4e89beb96f6401ed7e51bafdf0aac408f5a2c42f
This matches the name in core and is needed in preparation for
switching to SkinMustache
Additional change:
The JS variable htmluserlangattributes is renamed
"htmlUserLanguageAttributes"
Change-Id: I306ebb615f720852fb35c25ac240d6b802d05ecc
The renames made in this patch mirror the upstream change in
I3f7b2e5f07d03ac04ecdcba585194d619abe35e0 in preparation for adopting
the SkinMustache class.
Renames:
* 'data-indicators => 'array-indicators'
* 'html-bodycontent' => 'html-body-content'
* 'html-undelete' => 'html-undelete-link'
* 'html-sitenotice' => 'html-site-notice'
* 'html-catlinks' => 'html-categories'
* 'html-dataaftercontent' => 'html-after-content'
* 'html-userlangattributes' => 'html-user-language-attributes'
Adjustments
* 'html-debuglog' is moved into html-printtail for consistency with
SkinMustache in core
* html-printfooter is now a part of html-bodycontent for consistency with
SkinMustache in core
Note: the activity of removing html-printtail and html-headelement from
the templates will be done when we adopt SkinMustache.
Bug: T257630
Depends-On: Ibac9729eaa96a38bc730a0132b102a894f6a172d
Change-Id: Ie92bda7846767c3cc9a1937c96a7fc08415b1bae
Move the body of enableResponsiveMode method into the onBeforePageDisplayMobile hook.
Replace BeforePageDisplayMobile hook with BeforePageDisplay hook.
Bug: T254378
Change-Id: I63da1b67bf2b85c644e4af196bf894efc4797433
The parent method was deprecated in MW 1.32 and this override has been unused
ever since.
Bug: T255474
Change-Id: I811f7b3a6dba0bb1eebf36ed52e363affc8511d2
This means dropping usage of Skin::addToBodyAttributes method
from Skin to pave way for its deprecation/removal.
The parent method is no-op, so this will not create duplicate with
the hook
Bug: T255698
Change-Id: Ieeccdd9ec3fdb3e3fc1a3016cfa87e0b8364aa3a
As part as our move away from VectorTemplate construct html-printtail
using Skin methods. This copies the logic in
BaseTemplate::getTrail
Bug: T251212
Change-Id: I6a0c0b65f59fcf1f1550ec20d106f0143f681935
Begin our journey away from BaseTemplate by
moving VectorTemplate code to SkinVector. In future all
methods will live here but to lower risk, I've only targetted
the get method.
Bug: T251212
Change-Id: I58c2ff5edaacc2d5e45492c121cf0f87d08b623f
Add a menu button that toggles the panel's (also referred to as a
sidebar) collapse state. When the screen is wide enough, animate the
transition.
The menu icon from OOUI is copied into Vector to avoid two
ResourceLoaders modules (collapseHorizontal icon isn't ready for
inclusion in the OOUI icon pack and ResourceLoaderOOUIIconPackModule
doesn't support images).
Additional polish and collaboration is needed but this patch fulfills
the scope of its referenced task.
Bug: T246419
Depends-On: I8e153c0ab927f9d880a68fb9efb0bf37b91d26b2
Change-Id: Ic9d54de7e19ef8d5dfd703d95a45b78c0aaf791a
The collapsible sidebar adds a new JavaScript dependency and behavior to
Latest mode only. There are a number of ways of to make the deviation
but we think now is the time to start splitting by module.
This patch adds a new ResourceLoader module, skins.vector.legacy.js, and
moves the existing JavaScript into it. The old module, skins.vector.js,
has been given a currently matching index.js entry point that references
the collapsible tabs' files by reaching across directories. It's not
quite ideal as usually ResourceLoader modules and directory structures
strive for 1:1 correspondence but this patch makes the bold assertions
that it's better than a file copy, better than a new
"skins.vector.common.js" ResourceLoader module, more compatible than a
symlink, and the existing jQuery tabs implementation will eventually be
replaced in Latest mode.
A "Legacy" module was added instead of a "Latest" with the assumption
that active development should generally be considered "latest" and
Legacy an intentional distinction.
Bug: T246419
Change-Id: I9980403f1ee5897c27ac0331f0b51a5bcbdff778
Changes:
- Add the LatestSkinVersionRequirement requirement class, which lazily
evaluates the application state to get the version of the skin for the
request.
This majority of this class is taken from Stephen Niedzielski's
Vector\SkinVersionLookup class, which was introduced in d1072d0fdf.
- Register an instance of LatestSkinVersionRequirement and register the
'LatestSkin' feature that requires that requirement
- Re-introduce SkinVector::isLegacy and make it defer to the Feature
Manager
Bug: T244481
Change-Id: If6b82a514aa5afce73e571abdd8de60b16a62fa8
As described in the readme but not implemented until now, this patch
enables the skin version to be specified as a URL query parameter. This
is useful for testing both skin versions during development and on wiki,
as well as enabling sharing URLs with a specific skin (Vector) and skin
version (1 or 2).
Obtaining the actual skin version requires tying together three input
sources, WebRequest, User, and Config. It seems simple but it'd be easy
to botch. For this reason, a helper class to correctly interrogate them
and tests are provided.
Bug: T244481
Change-Id: I52d80942b4270c008d4e45050589ed9220255a50
This commit is singularly focused on adding a link to the sidebar for
Vector, logged-in users. It does the bare minimum to fulfill the
requirements of T243281.
Additionally, it will help to answer the question "Do we need to use
abstractions (other than maybe different templates) to separate Legacy
Vector from Vector" by intentionally leaving out any abstractions in
order to make it easier to compare with a follow-up patch
(Ib2ef15180df73360cc1de25b893e49d415d23e1a) which does use abstractions.
It is a good thing to question whether or not we need addtional
abstractions in VectorTemplate and if they will help us as unnecessary
abstractions can have the opposite effect and just lead to further
frustrations down the road.
Therefore, I urge you, the reviewer, to let me know your thoughts! If
abstractions are viewed as not making our lives any easier, the
follow-up patches may be completely discarded and that's totally okay
with me. :) I think it's a good think to talk about now though.
Important changes:
* The VectorTemplate constructor was changed to allow injecting the
config, templateParser, and isLegacy boolean (only the config was
allowed before this commit). According to MediaWiki's Stable Interface
Policy, "Constructor signatures are generally considered unstable unless
explicitly declared stable for calling" [3]. Given that VecorTemplate's
constructor is not marked as stable, it is justified to do this without
warning according to the policy.
* Due to the above, the 'setTemplate' method is no longer needed and was
marked as deprecated.
* VectorTemplateTest was made to adapt to the new VectorTemplate
constructor. Additionally, it now extends from
MediaWikiIntegrationTestCase which my intelliphense server can pick up.
I *think* MediaWikiTestCase is just an alias to
MediaWikiIntegrationTestCase [1] and MediaWikiTestCase file was renamed
to MediaWikiIntegrationTestCase in [2], but I'm willing to change it
back if there is pushback to this.
Open questions:
* What are VectorTemplate's responsibilities? To me, it acts right now
as a controller (because it echos the full HTML string from the
template), a model (because SkinTemplate::prepareQuickTemplate sets data
on it which it later retrieves through `$this->get()`), a presenter
(because it adds data tailored for a web-centric view), and a view
(because it renders HTML strings instead of letting the view/template be
solely responsible for that). Arguably, some business logic might be
mixed in there as well (because it checks to see if a User is logged
in/has necessary permissions to show x which my changes here add to).
This might not be a problem if we keep VectorTemplate relatively small,
but will it remain this way as we progress further in Desktop
Improvements?
* How do we write tests for VectorTemplate without exposing unnecessary
public methods? For example, if I want to test the `getSkinData()`
method to see what state will be sent to the template, how should I do
this? One option might be to use `TestingAccessWrapper` to expose these
private methods which is what
`VectorTemplateTest::testbuildViewsProps()` does. Another option is to
accept this method as public. Is there a better way? Keep in mind that
even with access to this method, there might be many things to mock.
[1] 0030cb525b/tests/common/TestsAutoLoader.php (L64)
[2] Ie717b0ecf4fcfd089d46248f14853c80b7ef4a76
[3] https://www.mediawiki.org/wiki/Stable_interface_policy
Bug: T243281
Change-Id: I0571b041bcd7f19bec9f103fa7bccdd093f6394d
Various skins may extend SkinVector (although none are in production)
(https://github.com/search?q=%22extends+SkinVector%22&type=Code)
This makes it clear we no longer support that behaviour.
Because of the skins that are extending SkinVector currently we have
not resorted to using the final keyword at this time.
Depends-On: Ie3759c2acbf53c628577f6b05cfed17e0998a6bb
Bug: T248399
Change-Id: I2af8b2930c80f888791247bdaa2ae1c80576317e
To have a clean break for upcoming changes we will duplicate
index.less into legacy.less and create a new module to clearly
separate new styles from old.
The preferred name however does come with some caching challenges.
Cached HTML served to anons will continue to load the style module
`skins.vector.styles` for a period of 1-4 weeks
Provided we are careful with our changes during this period this
should be okay.
Change-Id: If32b59036e5cd62cbb804944ca93fa1a101c5129
Thanks to the dependent change, the print logo is now provided
in core so we can remove the custom Vector ResourceLoader module
ResourceLoaderLessModule is replaced with a ResourceLoaderSkinModule
and gains the features capability replacing the need for
'mediawiki.skinning.interface' making use of the changes added in
6845912bcf1.
Note that for cached HTML both 'mediawiki.skinning.interface'
and skins.vector.styles will be loaded. We can avoid this
by renaming skins.vector.styles if necessary (but I'm not
sure if we'd want to do that)
Bug: T232140
Depends-On: I00899c16c0325f36b671baf17e88c2b5187b3526
Change-Id: I569e0d800e147eabc7852567acd140108613f074
The SkinVector implements IContextSource, thus it has to provide
the getConfig() method. Vector skin is currently using
it's own `vectorConfig` in one place and it passes the VectorConfig
to the VectorTemplate class. Sadly the VectorConfig is the same
thing what $this->getConfig() provides. There is no need to make
this code more complex by handling two different config containers
which at the end are the same GlobalVarConfig instances.
If we decide to handle VectorConfigs differently, let's thing it
through, for now we should simplify code and remove all uncessary
logic. Thanks to that, there is also no need to override the
setupTemplate().
Change-Id: I89c8a77f7d96f867c8c72e61f9e104e14d9512d9
Tthe SkinVector::steupSkinUserCss is deprecated and
we should use the ::getDefaultModules instead.
Change-Id: I359049d0f159a0dfc374d1c0124e8989aeb4182d
Looks like the SkinVector::getPageClasses is doing nothing, only
calling the Skin::getPageClasses and returning the output.
Change-Id: I5becaa1505d46cbf9a07b8bfe73ddf324b740876
With this change the constructor of the parent class gets called like if
there is no local constructor. This allows the ancestor classes to
initialize themself in their constructor.
Change-Id: I4ec1ec935afa25b59be4020fca9ac71f23e62ad0
