If the VectorSearchTreatmentABTest config variable is truthy and the
user is loged in, then pick the Core treatment (defined in the
mediawiki.searchSuggest RL module) or Vector's Vue.js-based treatment of
the search widget based on their user ID. If not, then fall back to
picking the treatment based on VectorUseWvuiSearch.
Supporting changes:
* Update initSearchLoader() in skins.vector.js/searchLoader.js to check
whether the body.skin-vector-search-vue exists
* Remove wgVectorUseWvuiSearch from the skins.vector.js RL module's config
* Update the performance-related metrics collection to check which
module is being loaded rather that use the above
Bug: T261647
Change-Id: Idc978392f5db14f0ae2b06ade0175fe534f4ae70
For language-in-header feature, edits the <header> element to
contain:
- page title,
- language selector
- tagline (siteSub)
- Indicators
These elements are associated with header/meta content so grouping
them inside one header element makes sense semantically.
Bug: T248761
Change-Id: Ief6c4936d1ebe381432369f8d86419da5f7c6cae
This allows better compatibility with FeatureManager (e.g. can use
requirements such as REQUIREMENT_LATEST_SKIN_VERSION). It will become
especially useful in I70277c1082a504fbd5f6023e9873e8071de7e35d and when
A/B testing search.
Bug: T270202
Change-Id: I3a063e0b085765ea1db3c4478fb30c11b0942b75
A new config flag wgVectorLanguageInHeader is added to allow
us to render languages in sidebar or outside sidebar, in the
header.
it defaults to false to allow for further development and to
not disrupt the status quo.
To accomodate the new menu, a new header is added based on the design
in Minerva to contain the heading and language button. The language
button is floated to the right.
The new menu is not styled. That exercise is left for the follow up
task T268241
No caching implications of this change, as legacy and modern
experiences remain touched without changing the default value of the
new config flag
Bug: T260738
Change-Id: I5af1522cac3831c1c833388461fe254c03191f65
html-user-language-attributes contains not only the lang attribute
but also the direction. There doesn't seem any value in regenerating
just lang attribute
Change-Id: I93e8081be1e81adf16d40bff54f0cc22c7155dd7
Seems less risky to fix this rather than revert, given a revert will
just mean more risk when we do it again, especially if the codebase
changes again and future manual rebases fail.
Bug: T271364
Change-Id: I7f63d6f07b6b715a9f31c83d572814da33ff2796
SkinMustache in core provides most of what is required for Vector to
generate its menus. In the interest of having a canonical source of
truth for menus across all skins, Vector should use this data.
To ensure the HTML generated is (mostly) the same after this patch to
prior, a few modifications are necessary:
* The data from core is decorated so that Vector can continue having its
own custom class names on menus. This is done using the
decoratePortletClass method.
* There is no support for a menu having a header representing the
selected menu item, as is currently the case with variants. This is
achieved via an extension to getPortletData. It's assumed that later
when variants are merged with languages, this can be removed.
* Menus are agnostic to how they are displayed, so we must continue to
add the is-dropdown template variable to drop down menus. In future we
may want to rethink our Menu partial to make this unnecessary in PHP.
* The portal-first class is redundant in the modern Vector as we can
use the first-child selector. Previously we introduced a class to
service the legacy skin where this rule doesn't apply as #p-logo is
the first child. However, the legacy skin can do this using a special
next sibling selector instead.
Bug: T268157
Change-Id: I5f7adc1840441b508ffee40139b85b64021789e6
Add event listeners and associated helpers to emit SearchSatisfaction
events via the `mediawiki.searchSuggest` protocol.
Bug: T257698
Change-Id: Ica040cd18d6c4bf8a1b1f607bb4647c7e8eb7108
By default the API uses location.host as the host, however during
development it is useful to test against production wikis
For example to test against English Wikipedia:
$wgVectorSearchHost = 'en.wikipedia.org';
Note: Links when clicked will not take the user to the target page, and
instead will take the user to the search results page with a link to
create the page.
The following config can be used to workaround that page:
$wgDisableTextSearch = true;
$wgSearchForwardUrl = "/w/index.php?title=$1";
Change-Id: I5fbac7f54844d7a9d6976007bc0d0ff9938b9f2b
These are no longer referenced in the code and artifacts of a time
when these were feature flagged.
Change-Id: Ia850ac9fb92033d82a333836ba7ff32312f042a7
Uses ResourceLoader's virtual config feature to get the config and pass
it down to Wvui's typeahead search component.
Disclaimer: I'm a typescript noob and am not sure if the
config.json.d.ts is correct although it seems to make tsc happy.
Bug: T260167
Change-Id: I2eced14c7df3b795b4de0e5149c2ca9fd598c7be
Creates a new skins.vector.search module that
replaces the searchSuggest module from MediaWiki core.
This module creates a new Vue app using the WVUI
search widget for the new search experience.
The legacy search input form is still retains on pageload,
and the new search kicks on search input focus.
In order to manage that transition, the legacy search
input is styled to resemble the new WVUI input, and the
new input is manually focused after the component mounts.
Vue is also added as a dev-dependency to help with
type-checking.
Other changes:
* the entry in skin.json is reordered alphabetically after
skins.vector.js
Bug: T264355
Change-Id: Ibb9561a77a14734297cb4d0ddcd415fc0750b45d
In preparation for using the data provided by SkinMustache class
we need to separate the opt out data from the rest of the sidebar.
The opt out data is specific to Vector.
Change-Id: I4461da92c1787d272bbf99e6644bdb9e6c388a68
The "Jump to navigation" link was removed as part of
Ic553fab3bde25769b103d899b92b3b694c00c384 which makes this div seemingly
obsolete (Volker pointed this out). It has a faily ominous comment above
it so might need more investigation if it will break anything.
Change-Id: Ida97cb640885dc2ea4dfda5b475672eb1ad973f2
Drop support for vectorMenu, vectorTabs and
vectorMenuCheckbox, body, menu selectors in preference
for standard selectors.
This change will impact a large amount of user scripts/styles but should
not impact any gadgets.
These classes were kept around for user scripts and styles however are not
needed internally. As we transition to a more maintainable skin menu
system, it is time to lose these selectors even though this will cause
disruption.
Vector now will use the mw-portlet class rather than the vector-menu
class in its own CSS styling, however it keeps the other classes to
allow differentiation of the different types of menu.
Changes to test: Previously the tests assumed all portlets were empty
when checking the classes. This is very rare, so its better to check
the classes of non-empty portlets, so several tests are updated
accordingly to drop the emptyPortlet class.
Bug: T262092
Change-Id: I1824335eb47d613c2a4804ec1f1106c0f4c16101
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
A new config flag is added that buckets 50% of users into the old
header and the 50% into the new header.
Bug: T249363
Change-Id: I8b4fa475f9cd7e61ad2989e2a1485e7e64c8ab3f
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
Drop max width feature flag.
Max-width will continue to not apply on special pages.
This saves us development effort by not needing to worry about
the case where the flag is not enabled. This flag is not false
in any production wikis.
Change-Id: I7ace4046e6b93ce63dd804da32e576a709485bfb
This moves the header, navigation, sidebar, and article toolbar to be
before the content in the DOM. As a result, a lot of absolute
positioning logic can be removed and styles can be simplified.
Note that although the sidebar was moved from the header into the
workspace container allowing it to de-absolutely positioned, its
absolute positioning was kept intact as it has a fair amount of
complexity that should be handled in a separate task.
To activate, set `$wgVectorIsSearchInHeader = true;`
Changes that could cause concern:
* The "jump to search" link was removed as the search is now much
earlier in the DOM and I questioned the value of keeping this. However,
it can be added back in if this change is contentious.
* A "jump to content" link was added to account for the new DOM order.
* Because the sidebar was taken out of the header, users will not be
able to tab from the sidebar button into the sidebar without additional
tweaking (e.g. should we add JS to enable this?). It was deemed that
this work can be saved as a follow-up task.
* I applied `overflow-y: auto` to the `mw-page-container` because the
header's top margin was collapsing and caused whitespace to appear
between the viewport and the header. Alternatively, we could apply a top
padding to the page container and remove the header's top margin. I went
for the simplest solution but am open to alternatives.
* I left the footer as-is in this patch to minimize risk. It might be
cleaner later on to move the footer inside the workspace container which
would leave only one workspace container.
Bug: T261802
Change-Id: Ic553fab3bde25769b103d899b92b3b694c00c384
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
I177dad88 introduced the skin version user preference field and
associated configuration values. Per T242381, the field is to presented
as a checkbox with the implied storage type of a boolean where a string
is needed. A PreferencesFormPreSave hook handler was added to adapt
values of either data type to the other. While this was a neat solution
to a minor nit, the adapter's implementation is incompatible with the
GlobalPreferences extension as the PreferencesFormPreSave hook isn't run
whilst saving global preferences.
Rather than adding an equivalent hook to the GlobalPreferences
extension, create a custom field based on a checkbox with the adapter
included. This allows us to:
- Separate the business logic concerned with preserving the user's
VectorSkinVersion preference if they've simply disabled Vector from
the adapter
- Simplify the adapter's implementation
- Forego adding hooks to the GlobalPreferences codebase
Additional changes:
- Replace repeated string literals with equivalent constants in
tests/phpunit/integration/VectorHooksTest.php
Bug: T258493
Change-Id: I628435a4ad676f55534191b8c10147be28be5d73
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
This hook is run on every page. The SkinTemplateNavigation hook
counter intutively is run only on pages which can exist. I think
it's clearer if we only use SkinTemplateNavigation::Universal hook
and keep the logic for when it runs inside our own code.
Bug: T255319
Change-Id: I0835074a6cadf6e9bdcc45299de37dd9328bf9b2
The feature manager abstracts away how a feature is enabled from the
consumer of that feature. Accordingly, replace direct instantiation of
SkinVersionLookup with usage of the Vector.FeatureManager service.
Supporting changes:
- Add Vector\VectorServices, a simple wrapper around
MediaWiki\MediaWikiServices that allows us to both document and
type-hint services specific to Vector
- Add Vector\Hooks::isSkinVersionLegacy to minimise repetition
Additional changes:
- Make the MakeGlobalVariablesScript hook handler return early if the
user isn't using the Vector skin like the other hook handlers
Bug: T256100
Change-Id: I93b5ef39802323c7ac658af8fa7cc312fff68aa7
The wgVectorDisableSidebarPersistence config variable is only required
when the user is logged in. Don't send it to the client when the user is
logged out.
Bug: T255727
Change-Id: I27eda8c18b438ef3251b2c07a0ed8b0cef6cae64
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
Following on from I9445d5c, align the @package annotations in
the Vector\FeatureManagement namespace and subnamespaces.
Bug: T248399
Change-Id: Icd287a52d149123bca5d9f0c55154f932f55148e
Add onSkinPageReadyConfig hook that overrides module after page loaded
The new module is currently empty pending further work in the
feature branch.
Depends-On: I0dc38e74052027f26a70d58b5f520e5830e0d55d
Bug: T257706
Change-Id: Ib6c8f890fb3d6e751f5f01a6576614b9cc9b440c
Hooks are global, unlike the old Skin addToBodyAttributes() method, which was per-skin.
Follow-up to a04fc71f30
Change-Id: Ic8a581256c4775ba45b2c5188d678751ba9c7023
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
* `mw-content-container` now wraps the footer (as well as the content)
because we want the footer to match the content width at all times and
to expand with the content when the sidebar is closed (at small viewport
widths or when on history/special pages)
* `mw-footer-container` margins were replaced with padding to avoid
issues with margin collapsing.
* Applied a white background to sidebar to handle the case of the
sidebar overflowing the `mw-page-container`. When that happens, we at
least want the text in the sidebar to be legible.
* Closely related, `mw-page-container`'s `overflow: hidden` style was
removed to prevent `mw-page-container` from cutting off the sidebar. The
purpose of this style was make it appear as if the sidebar was being
hidden by `mw-page-container`, but tweaking the sidebar's translation
animation to achieves this effect as well.
Bug: T257518
Change-Id: I89edf89b2ac4abe2053f0c9b366f143133ff420f
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
- Remove unused html-prebodyhtml template variable
- Don't use html- prefixed variables as if statements - this is
not supported by Mustache. (set to '' to confirm)
Change-Id: I0c860b4b2277e1796afa3e482cf61e95ef4fb342
In 1.36 we will be removing this class and adopting SkinMustache.
Signal to developers that this is considered an internal class
as notice of that intention.
Change-Id: Ib62a5352a73eb6189ba85f4034da1b0857fd31aa
- Creates a new user-preference called 'VectorSidebarVisible'
which stores the sidebar hidden/collapsed state for logged-in
users.
- Updates that user-preference on the client whenever the sidebar
is expanded or collapsed.
- Refactors the sidebar related javascript into a separate file.
Bug: T255727
Change-Id: Ib1ce934f3646cd8feebf0d3b15c38b5b969ec957
VectorTemplate was overriding msg() in QuickTemplate
in a way that broke the method's
contract, and changed the return type in an incompatible way.
Note while getMsg would also work here, this will reduce the
risk in the upcoming I4e89beb96f6401ed7e51bafdf0aac408f5a2c42f
change.
Change-Id: I95e4e7dcd5ed353917d2706c78663f748775b365
This patch closely follows the desired guidelines/desired
styles Alex Hollender has put forth in his prototype, but uses
multiple containers to achieve this look since our DOM order/structure
is different than the DOM structure in the prototype. The following
containers are used, but unlike his prototype, they are sometimes used
more than once:
* Page Container: Contains every other container and limits the overall
max-width of the white part of the page.
* Workspace Container: Contains the sidebar and content container. The
sidebar is displaced ~30 pixels to the start (left) of the workspace
container at all times.
* Content Container: Contains the content. The max-width of this changes
depending on whether you are on a special page/history page vs. other
pages.
* Article Toolbar Container: Contains the article toolbar. The max-width
of this is always the same as the max-width of the article content as we
don't want the toolbar to move when going from the article page to the
history/special page.
Changes to be aware:
* To test locally, `$wgVectorLayoutMaxWidth = true;`. This design is
temporarily feature flagged and defaults to being "off".
* Note that layout-max-width.less is a temporary file made to meet the
feature flag requirement of T246420 (intended to derisk the deployment).
After the deploy, we should merge most if not all of the rules into
layout.less where the max-width design will become the default.
* Per Jon's code review comment, I have relaxed the indenting of
skin.mustache to make the diff easier to reason about. If desired, the
correct indenting can be achieved in a (much less risky) follow-up
commit.
Bug: T246420
Bug: T153043
Change-Id: Ie49f629bc705850c6996164a516957476c034048
Move the body of enableResponsiveMode method into the onBeforePageDisplayMobile hook.
Replace BeforePageDisplayMobile hook with BeforePageDisplay hook.
Bug: T254378
Change-Id: I63da1b67bf2b85c644e4af196bf894efc4797433
Removing presentational `<div class="visualClear"></div>` in process.
Only leaving in for legacy/layout.less and adding comment for future
evaluation if clearfix is continuously needed. In modern we've
got all parts of Vector in our awareness without featuring `float`s.
Bug: T254195
Depends-On: Iddf8f1dffc2d30299b89b3f4966b7fe8ee63090b
Change-Id: I673c28c2d7da4f96c31121d9aec6558e390de24e
* Moves the sidebar into the header so that user can tab directly from
the sidebar button into the sidebar (when open).
* Because the sidebar is absolutely positioned inside the header and the
header applies a top-margin, we need to adjust the top position
calculation for the sidebar.
* Removes the checkbox from the accessibility tree through CSS `display:
none;` instead of HTML attributes.
* Given that the checkbox is not able to receive focus, the
`#mw-sidebar-checkbox:focus ~ .mw-header .mw-checkbox-hack-button` CSS
rule is obsolete and moved instead to the label button. An additional
outline: 0 rule was added to remove the dotted outline that Firefox
applies.
* Makes the "Skip to navigation" jump link point to the sidebar now.
After the sidebar has been tabbed through, the rest of the navigation
can be tabbed to.
Bug: T246420
Change-Id: I981da3650854568bb9cfbf3c6c59e7625e7d094c
Move the VectorBeforeFooter hook code into its own function
This adds consistency with the getMenuData function.
Change-Id: I6483fd200747852246e9cc587f57b1a21a2f0a44
Hide checkbox from AT by adding `aria-hidden="true" to checkbox
and disable tabbing in via tabindex="-1". Both attributes are temporarily
added and might be removed when the checkbox receives `display: none` in
upcoming patch.
Add
- `role="button"`,
- `aria-controls` and
- `tabindex="0"
to button turned label for ATs via `role` definition, make it focusable
and use `aria-controls` for connection to sidebar id.
Bug: T254851
Change-Id: Ife287fc8c6e0d2aee5facf42d5d4308dea918ee3
Even if we were to use it, we might sign up for side-effects when
more than one checkbox hack is used per view.
Change-Id: Ic31e48a4bf3e39e903300acd522b7af0391fafa9
The exact search input element as target focus the searchbox. It saves
AT users a user action, while still remaining read out due to connected
label in screen readers.
Change-Id: I83003fbbb5b6903038d1ca8b323302cb8f52e841
In preparation for moving the languages out of the sidebar (T256023)
and to facilitate experimentation in the Universal Language Selector
extension, we should separate the data from the languages portal
from other portals.
Note the languages portal has traditionally appeared at the bottom
of the sidebar, so this can be done safely in the same way we
introduced data-portals-first
Change-Id: I4be06278ec256cc55aee36c2946339fb49d21b46
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
Following-up I00b4d2fd795195cd9c8add650a3b3cafdced5465
it's now time to settle on `.vector-menu*` classes.
Change-Id: I8669d402b6b757cd5bf59a5e0df377b6023b0700
Because the method will soon be deprecated, and by calling it, it can
override hook modification on 'nav_urls' and 'feeds' arrays.
Bug: T253416
Depends-On: I3d7aa93bea47131371e58d2cd2e2d81fd1d7421e
Depends-On: Iab67e1430d55ef9bbf01bfb4e64dd5e36b954eda
Depends-On: I05dae74b5071ffc668c3569566239052575edfb9
Change-Id: I76e3bea5403e3a6994b911017e2254307588499c
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
`[role]`s are retained for now as they are harmless although
redundant when added to HTML5 elements.
`role="contentinfo"` on `footer` is a special case to address
a known VoiceOver bug.
Bug: T66477
Change-Id: I540d9a41fc7fd580c5d61b90480e8745ae145850
Fix the icon button directionality in right-to-left languages.
Previously, the button was hardcoded to support left-to-right only.
- Replace the skin.vector.icons' `.mw-ui-icon-wikimedia-{name}:before`
`selector` in skin.json with a placeholder, `{name}`. I don't think
this selector should be needed but it seems to be erroneous not have
one. I believe this issue of wanting a null selector was encountered
in Minerva or MobileFrontend but am unable to locate the past
discourse.
- Add check and unchecked menu button selectors to skin.json that set
the appropriate background image. This shards some of the styles out
of Less and into ResourceLoader-land but it's worthwhile.
- Revise the name of horizontal collapse icon to describe its form not
function, "collapseHorizontal" to "chevronHorizontal". This has been
an established convention that was missed a couple patches back.
- Add a flipped chevronHorizontal for RTL. I used Inkscape to do the
flip and tried to match the style of the original by hand. Feel free
to edit further.
- Drop the now unnecessary icon flipping JavaScript and initial Mustache
class. This enables a real CSS-only solution for the icons.
Bug: T246419
Change-Id: I60f65b3c595bf18d309b667d9a0b066691b90c97
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
It's advisable to rely on default icon color and set it's color
by opacity. With that we've got a better playing field to respond
to user interaction with icon color changes, not relying on
several icons at once for different states and also being
backwards-compatible for a variety of browsers.
Change-Id: Iaff869774007ed962104d704103f0392a3516f4f
Bug: T246419
The sidebar in Latest mode supports collapsing and expanding (or hidden
and visible). Initially, expanded is wanted.
Bug: T246419
Change-Id: I0777f58fb9441aa834a29afa1b52185ad1b58ca7
Keys prefixed with msg- should relate to i18n keys. Fix the existing
template variables where that's not the case.
Change-Id: I33587a09a9803a3667eaeea95ce3e1753439f413
We have conventions about template variable naming.
* Drop camel case
* Prefix message keys with `msg-` followed by message key name
* Prefix HTML with `html-`
Change-Id: I34f04f3d217dc8caa4dcc29e60058951a6555b1e
Using 'skin' as entrance point files, similar to already existing
convention in MediaWiki land with 'skin.json' or 'SkinVector.php' as
example in Vector skin. Replacing Apache inspired 'index' convention.
Also renaming legacy to 'skin-legacy' to be clear on file base that it's
a modification of 'skin'.
Bug: T249073
Change-Id: Ief1c469724d4ffe238d307407c3ddb46f2e1abfa
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 .menu class historically only needs to apply to dropdowns.
the .vectorMenuCheckbox is inconsistent with the other classes on the
menu so we should begin its deprecation.
Bug: T253329
Change-Id: I00b4d2fd795195cd9c8add650a3b3cafdced5465
The opt in link is part of the sidebar in the current modern version
of vector. The legacy sidebar has been split out and frozen. For this
reason I think it makes sense to move the styles for the button into
the sidebar stylesheet.
The class name is renamed to reflect the hierarchy. We are not using
BEM but we are using hyphens to describe location so the newly proposed
names `mw-sidebar` and `mw-sidebar-action` seem more applicable than
the previous generic class names.
This doesn't impact caching given anonymous users cannot see the opt
in/out link.
Change-Id: I2991e941592a0678f40c73bb6daeab71ada6b7c3
Use SkinTemplateNavigation hook instead and copy the collapsible
behavior to the menu function
The code inside getSkinData that checks VectorUseIconWatch is
redundant as it duplicates checks already inside
SkinTemplate::buildContentNavigationUrls
It is enough to simplify check whether watch or unwatch is
present in the array.
Bug: T251212
Change-Id: If6b10b0ddcbd4b21dd13a2813e60b604c3a23415
Split out the sidebar in advance of significant Latest mode only
changes.
The Less split was made by adding a new Sidebar.less file that imports
the Legacy implementation.
The Mustache split was made by file copying the existing implementation
into a legacy/ subfolder. Both the Legacy and Latest implementations
were then stripped of templates not applicable to their modes.
These changes are aligned with the splits for JavaScript.
Bug: T246419
Change-Id: Ib82769ea16bfc09efe8a088220aab55bdde0d381
* Standardise the menu markup. This means all menus in Vector will now
be wrapped in a div and will have a heading.
* All menus now have the vector-menu class. Styles specific to personal tools
are moved to layout since these are concerned with placement.
* The ul class will always have menu class.
* emptyPortal class is generalised into vector-menu-empty for consistency
with other classes and moved from common.less into Menu.less
* Standardise hooks - BaseTemplateAfterPortlet can now be run on any
menu.
Changes to HTML:
* lang and dir attributes are moved from the h3 up to the div element
.vectorTabs, .portal(s) and #p-personal now has hidden span element inside h3
* for non portals ul.menu" is now wrapped in a div.vector-menu-content
This change does impact the following CSS selectors which will need to be updated:
I see no matches for these selectors in code search.
```
#p-variants > ul
#p-namespaces > ul
#p-personal > ul
#p-views > ul
#p-cactions > ul
```
Using global-search.toolforge.org I see one match
for p-variants, 26 for p-namespaces, 30 for p-personal,
36 for p-views and 7 for p-cactions. I see this as acceptable
breakage provided a user notice is sent out which it has been
(T252447)
Bug: T249372
Change-Id: Id59234aa6b822a24848386bdc04d8d7ed37ca145
This unblocks reverting HTML5 elements.
A longer term fix is brewing in
If6b10b0ddcbd4b21dd13a2813e60b604c3a23415
Change-Id: I85659d22dc2f2a803c2a7b1e57033b18900353df
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
Should have been removed in
I2549af3e24e5d51c09e9a88ca50a0d9b2e154c3f
Presumably got readded while addressing a rebase conflict
Bug: T249372
Change-Id: Iaff98d8b4770e82277463f39800dd7571e1ed50b
Hope you enjoyed the pun, as this is an unfortunate bug to have
slipped into production.
While previously we breaked after finding the selected items
prior to I098e6921e8f7ef65dacacf09b9c25f70c945e58e we shouldn't do
that any more as we're using the same loop to iterate through and
generate our items!
Bug: T251521
Change-Id: I92c28e8fba684d7977dfd207ff939ac07a7a07af
To complete the refactor, the Portal is also refactored
as a Menu using the getMenu function.
An old code path supporting portals outputted by hooks with
strings is marked as deprecated to simplify this code in future.
array-portals-first -> data-portals-first (the value is not
an array)
Changes:
* $this->getLanguages and $this->getToolbox() always returns an array (see BaseTemplate)
but we previously supported portals made using raw HTML. Let's move away from that
behaviour and deprecate it.
* Hooks are moved into buildSidebarProps and marked as deprecated where possible
(SkinTemplateToolboxEnd). SidebarBeforeOutput can be used instead.
Bug: T249372
Change-Id: I2549af3e24e5d51c09e9a88ca50a0d9b2e154c3f
Also remove superflous `role` attributes in modern Vector. Only leaving
`role="contentinfo"` on `footer` to address a known VoiceOver bug.
And leaving `role="main"` in legacy Vector due to possible old
Internet Explorer and screenreader combinations, which we can
test specifically for with this separation.
Bug: T66477
Bug: T252090
Change-Id: I422ef2a4cf7aa94e5cac4d95efb4b2d2c8c6617d
The classes were recently changed so provided this is merged before
next branch cut no need to worry about cached HTML.
Bug: T249073
Change-Id: Ib20c7a359bda858df89ebb245e682d321dd5acd0
the PersonalMenu should be generalised. In future we will use it as
the template for all menus
Bug: T249372
Change-Id: Id1c43d2e9eefef1d7aec45f0137e27f10ad935df
The get method while well intended, adds an unnecessary level of
indirection to our code and can't be easily typechecked. Using
the methods directly (which if you review core code, these all
directly map to those methods) makes the code much more of a pleasure
to read.
Bug: T251212
Change-Id: Ibb2a69801d3b004a0a174308d3ac04d1d228807c
The opt-out link was missing a tooltip which is important for
accessibility and to help people gain more context as to what it does.
Bug: T250093
Change-Id: Ie6cbaf5c941615d1662700415b8f1823987a563d
Changes to support feature:
* ResourceLoaderSkinModule logo features are dropped
* New layout provided given the fork in layout between legacy and new.
* Legacy sidebar styles now pulled out
* breakpoint styles are not carried over from legacy Vector
The new Vector layout for now has one breakpoint.
Changes to storybook:
* The storybook script now pulls down image assets so that the logos can
be shown in storybook. The script is adjusted to make use of a static folder to
serve these images.
Note:
* The legacy mode is not touched as part of this patchset.
* The personal menu is unaffected by this patch and is out of scope.
* The alignment issue is noted, but will be solved at a later date.
* Changes to portal are out of scope.
* Adding storybook for modern descoped, given its not possible to load
both legacy layout and modern layout inside a storybook at current time.
Sample config:
$wgLogos = [
'icon' => 'https://di-logo-sandbox.firebaseapp.com/img/globe.png',
'tagline' => [
'src' => 'https://di-logo-sandbox.firebaseapp.com/img/tagline/en-tagline-117-13.svg',
'width' => 117,
'height' => 13,
],
'1x' => 'https://en.wikipedia.org/static/images/project-logos/enwiki.png',
'wordmark' => [
'src' => 'https://en.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.svg',
'width' => 116,
'height' => 18,
],
];
Coauthor: Aron Manning
Bug: T246170
Change-Id: Ibc4b055150761388a6b78f9127da342c451ce0e7
de76ab5 added the config,
`$wgVectorDefaultSkinVersionForExistingAccounts`. Its usage in
`Hooks::onUserGetDefaultOptions()` was invoked not only for existing
accounts but anonymous users _as well._ This is a bug, due to my own
misconceptions about the hook, that went against both the config's name
and its documentation.
Unfortunately, user sessions are unavailable in
`Hooks::onUserGetDefaultOptions()` so it does not seem to be possible to
determine whether the active user is an anonymous or existing account.
This patch drops the hook and centralizes all version determination
logic in SkinVersionLookup::getVersion(). SkinVersionLookup requires a
the active User object and can make the anonymous / existing account
determination by checking login state.
The issued was identified while responding to review feedback given by
@polishdeveloper / @pmiazga in
I52d80942b4270c008d4e45050589ed9220255a50.
Bug: T251415
Change-Id: I7982b4c34283ba81d0232ee6f501c44cf0a74b98
FeatureManager::registerRequirement registering an instance of
SimpleRequirement with ::registerComplexRequirement was awkward.
Changes:
* Rename FeatureManager::registerRequirement to
::registerSimpleRequirement, which is exactly what it does!
* Rename FeatureManager::registerComplexRequirement to
::registerRequirement
Bug: T244481
Change-Id: I612af959cee4cdcd0bdcda51a81b86ed61ee2e16
In future we'll convert VectorTemplate to SkinVector. In preparation
for this we create wrapper functions getMsg and getConfig so that
the interface for VectorTemplate is more similar to SkinVector.
Bug: T251212
Change-Id: I1f839d3147499d67e1c7cf71cf934bb7d20beedb
Move all the sidebar generation logic into a new VectorTemplate
function.
The name mirrors the function name in SkinVector. In future,
( I7a14f74728703c50874935e9d77b35ad9434b436)
we will move this code into SkinVector where it will be able
to call parent::buildSidebar()
Bug: T251212
Change-Id: I0a4838120f6ffd3d3d798f876e3463a3f16af95b
- Removed `#mw-head-base`.
- `#mw-page-base` -> `.mw-header-placeholder`.
- `#mw-head` moved down to `@height-header` to make space for new header.
- Content moved down to `@height-header + @height-tabs`.
Added Less variables:
- @height-header: 69px;
- @height-logo-icon: 50px;
- @height-header--inner: 54px;
- @height-tabs: 2.5em;
Style splits:
- Added Header.less
- @height-header: 2.5em;
Bug: T246170
Change-Id: I07280c4c7a2054439153e76359f712281049df0b
Does not add `.mw-footer-*` CSS classes to `ul#footer-*` and
`li#footer-*-*` to save bandwidth.
NOTE:
- The former id selectors can be removed form the styles after the varnish cache
is updated with the new DOM.
- The modern version still uses the legacy layout at this stage.
Bug: T248137
Change-Id: Ica9f8c43617c624648fa12dc86ebb3daa10f0409
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
Going into the new version of Vector, we'll want to rewrite the layout
rules entirely and decouple the sidebar from the logo.
To prepare for this we will move the layout file into a legacy folder.
We also separate Sidebar styles needed for the legacy mode from the old mode.
This will allow us to make changes to the header in new Vector without having to touch
or test the legacy codebase via a new file layout.less and using the existing sidebar
code.
Bug: T246170
Change-Id: Ieb4f8f2b2f0e4f48d76a210ab30acd08e3e83bcb
In new Vector, the logo will no longer be present, so we need a more
future proof way of determining what the first portal is.
A new class `portal-first` is added (no decision about adopting BEM
has been made yet).
Cached pages will continue to use the old selector for now.
Change-Id: I6ac4493bb1f63686b48ff0c22b18d50d9fffb51d
VectorTemplate has various functions that repeat themselves, only
differing in their choice of names.
This refactor begins by focusing on the personal menu and introducing
a generic getMenuData function. Hardcoded `p-personal` is replaced with
an `id` template key and `msg-label` is renamed `label`.
Future patches will simplify VectorTemplate by using this new
function.
You'll note the resulting PersonalMenu.mustache file is identical
to VectorTabs. These will be merged in I098e6921e8f7ef65dacacf09b9c25f70c945e58e
Bug: T249372
Change-Id: I5ae44a1008b065381eeff93f9fa625be5c5a9de9
The Mustache templates were a bit of a mix of tabs and spaces. I like
spaces personally but dislike inconsistency within the project more. Use
tabs.
Change-Id: I22b777489d7281b42d1ab1981b47b2a71d3b639a
Adding `margin-top` property only with sibling selector for now
until cache is cleared.
Also removing already inherited from
`.mw-body-content` properties `position`,
`font-size` and `line-height` of same value.
Bug: T248761
Change-Id: I1ea5e08927a96ac69c1b65f248ae0420968b4d00
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
The new template 'index.mustache' drops the Navigation
template partial (component) - instead embedding it in
the master template given its lack of usability.
Note to reviewer: Please ensure index.mustache and
legacy.mustache are identical.
Change-Id: I580fe55ca6aeb35dae8afee41e87172a5a729c7b
It is not the most useful of components and adds an additional layer
of complexity similar to multiple inheritance chains that we find in
Object oriented programming.
I suggest we use index.mustache going forward for laying out the different
components and use components/template partials for reusable components.
Change-Id: I6fd5fe1c3d3826d737ccd8ed5a38890305664876
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
Address some feedback from I7a2cdc2dfdf20d78e4548f07cf53994563b234b3:
- Miscellaneous documentation improvements.
- Add a false case test to `DynamicConfigRequirement->isMet()`.
Bug: T244481
Change-Id: Ic5637f42da755f871c5a6d545e14effd3ac8c670
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
* Content.mustache doc param order was modified to follow the html structure
order (with html-dataAfterContent at bottom ). `html-userlangattributes`
was added to param documention since it was absent.
* Footer.mustache doc param order was modified to follow the html structure
order (with `html-userlangattributes` placed after `html-vector-before-footer`)
* Navigation.mustache was modified to remove `html-sidebar`,
`html-navigation-left-tabs` and `html-navigation-right-tabs` params from
doc since those are absent from Navigation.mustache.
* `data-namespace-tabs`, `data-variants`, `data-page-actions`,
`data-page-actions-more`, `data-search-box`, `data-sidebar` params were
added in Navigation.mustache documentation since those are present.
Bug: T243281
Change-Id: I321f8d61ad0305f508521e8d4d805e846103b357
We expect the vast majority of requirements and features to be defined
in services as possible. However, there are some "complex" requirements
that require additional application/HTTP request state. Unfortunately,
service wiring is done before some of that state is available.
I65702426 attempted to work around this by requiring clients of the
Feature Manager to pass that additional state on every interaction with
the system. Those complex requirements would then select the parts of
the state that they required when it was required. However
implementations of \IContextSource are God objects and their use should
be limited.
Whilst reviewing I65702426, Stephen Niedzielski mentioned that the
application state being available is a requirement. This remarkably
simple solution:
- Keeps the Requirement interface and FeatureManager API free of God
objects;
- Is true to the nature of the Feature Manager - it makes clear and
centralizes the various checks for application state being available
across the codebase; and
- Inject a Requirement implementations' dependencies at construction
time
It just so happens that the $wgFullyInitialised variable flags whether
the application state is available...
Changes:
- Add the the FeatureManager\Requirements\DynamicConfigRequirement class
and tests. The DynamicConfigRequirement lazily evaluates a single
configuration value whenever ::isMet is invoked
- Register an DynamicConfigRequirement instance, configured to evaluate
$wgFullyInitialised while constructing the Vector.FeatureManager
service
Bug: T244481
Change-Id: I7a2cdc2dfdf20d78e4548f07cf53994563b234b3
Lift the mists of confusion by checking that all JavaScript types align.
No ignores! This is the JavaScript equivalent to Phan.
This patch adds the necessary infrastructure for verifying typing and
fixes the few flaws found.
Bug: T239262
Change-Id: I2557471421196ea46cd13dfb786a52968fbfcc97
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
This allows us to insert HTML underneath the first portal or wrap
the portal in a containing element in future if we want to target
additional CSS to it
Change-Id: Ied28d95407b8d59fc819bb07a2cce3242bd93088
FeatureManager::registerRequirement established the interface for a
requirement: its name and whether it's met.
However, the Feature Manager also needs to handle scenarios where a
requirement needs additional context before it can be considered met.
That context may not be available when the application is booting, e.g.
checking if the user is logged in; or the logic is complicated enough
that it should be under test.
Changes:
- Add the Requirement interface and update FeatureManager to work with
implementations of it
- Maintain B/C by constructing an instance of a the SimpleRequirement
DTO
Bug: T244481
Change-Id: Id95d9e5d7125492968d0e15515224aadbc3075f8
I735fd640 bumped the required MediaWiki version to 1.31. That version
dropped support for PHP 5.x.
Wherever possible, update FeatureManager's methods to use PHP 7.0.x's
scalar and return type declarations.
Bug: T244481
Change-Id: Ib5636d0ec5ec7f0c93b5b3317a12635668b589e2
Separate value type from value groups, to clarify that e.g.
something coming from a hook does not mean it should not have "html-".
Change-Id: I52fdd6995241e1d64f23fd8154997de813f674f4
As was noted in https://phabricator.wikimedia.org/T244481#5859513, the
term "set" doesn't seem natural. Piotr Miazga (polishdeveloper, pmiazga)
and Nicholas Ray (nray) suggested a number of good replacements,
including "requirement." Serendipitously, this term is already used in
FeatureManager's documentation.
Bug: T244481
Change-Id: I559c2d4149db69235cdd4bb880697deb1a145743
CHANGES to index.mustache:
Changes can be understood by looking at the diff of
the file stories/skin.stories.js
The additional changes in stories folder export the data passed to
those templates. A new file is used as exporting any variable in a
file suffixed stories.js will be assumed to be a story entry.
The changes to index.mustache are as follows:
* html-footer is replaced with data-footer and Footer
component is rendered via template partial
* html-navigation is replaced with data-navigation and
Navigation component is rendered via template partial
CHANGES to Navigation.mustache:
Changes are best explained by looking at the diff to
stories/navigation.stories.js
and navigation.stories.data.js
* html-personal-menu is replaced with data-personal-menu
Bug: T245456
Change-Id: Ie96e92447a932b8a7f3844df277a1d31a2af423c
Sections can be nested. Using rendering/skin as the parent
means that Vector's skin preference will always come straight
after the skin preference and before Popups.
A change in core is needed to update the selector for the element which
shows/hides the subsection as well as provide a generic message key to
replace the one inside this repository.
Note: If the "Vector" specific heading is needed, we can achieve this
with a little more work but that is a conversation for another time.
Depends-On: Idd06bcfe7935e16732a6a95c1253dbf95c8aca2e
Bug: T246162
Change-Id: I4be9764ddca186e5bfd493678afd62d446072e8f
For better encapsulation and future possibility to create Skin API
we have to stop accesing properties and start using getters/setters
instead. Once we start doing that we should be able to provide a
clear interface for our templating system.
Bug: T246161
Change-Id: Ib3539b1e3bc12341c79913af3c95acad8619cff4
Moving all the templateParser calls to one function
so its easier to see how the template is composed.
The diff of changes to the stories folder highlight
the internal changes which are:
* html-portals replaced with html-sidebar in main template
* new Sidebar template added which outputs to html-sidebar
* Mention of "MainMenu" replaced with better understood "Sidebar"
This is precursory work to adopt templatePartials
Change-Id: I6b2196e39087f818e774d04b2d1b9ab8cb8816a1
Add a Vector-specific user preference to Special:Preferences for
toggling skin version, either Legacy Vector or the latest Vector.
The presentation of the new preference section and the default values
for anonymous, new, and existing accounts are configurable via
$wgVectorShowSkinPreferences, $wgVectorDefaultSkinVersion (to be used by
the feature manager in T244481),
$wgVectorDefaultSkinVersionForExistingAccounts, and
$wgVectorDefaultSkinVersionForNewAccounts. These configurations default
to the fullest experience so that third-party configuration is minimal.
See skin.json for details. The configurations are each tested in
VectorHooksTest.php.
When presentation is enabled, the new preference appears as a checkbox;
enabled is Legacy mode and disable is latest. There are a number of
unfortunate details:
- Showing and hiding a checkbox is supported by OOUI. Showing and hiding
a whole section (Vector skin preferences, in this case) is not so this
additional client JavaScript functionality is added in Core (see
Iaf68b238a8ac7a4fb22b9ef5d6c5a3394ee2e377).
- Stylization as a checkbox is wanted. However, the implied storage type
for OOUI checkboxes is a boolean. This is not wanted in the event that
another skin version is added (e.g., '3' or 'alpha'). As a workaround,
the preference is converted from a boolean to a version string ('1' or
'2') on save in Hooks::onPreferencesFormPreSave() and from a version
string to a checkbox enable / disable string ('1' or '0') in
onGetPreferences(). There a number of test cases to help cover these
concerning details.
Documentation for overriding the skin version as a URL query parameter
is provided in anticipation of T244481.
Bug: T242381
Bug: T245793
Depends-On: Iaf68b238a8ac7a4fb22b9ef5d6c5a3394ee2e377
Depends-On: Ifc2863fca9cd9efd11ac30c780420e8d89e8cb22
Change-Id: I177dad88fc982170641059b6a4f53fbb38eefad6
With complex additions to Vector's codebase like the Desktop Improvement
Program upcoming, it's important that we have a shared, intuitive
language to talk about features and their requirements. Centralising
the registration of features and creating an API satisfies does exactly
this.
This change introduces a greatly-reduced version of Piotr Miazga's
(polishdeveloper, pmiazga) original proposed API and associated
scaffolding classes for feature management in Vector, which itself was
based upon his work in MobileFrontend/MinervaNeue. This is done to
establish a foundation upon which we can build the more sophisticated
parts of Piotr's proposal in a piecemeal basis, thereby minimising risk.
Distinct from Piotr's proposed API is the ability to register sets and
features that are always enabled or disabled.
Additionally:
- A Vector.FeatureManager service is registered but not used
- A list of proposed immediate next steps is included
Bug: T244481
Change-Id: Ie53c41d479eaf15559d5bb00f269774760360bde
The renderNavigation function provides an unnecessary level of
abstraction which I'd argue hurts readability of these functions.
Let's remove it.
Change-Id: Iad1c4db606404fecf4d5ae98981df9f05d3f661e
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
- Update package.json with the new dependencies.
- A script storybook.sh pulls down CSS and LESS imports from external
dependencies. This copies the approach taken in Popups and MobileFrontend.
- Icons from external repos are maintained within the repo in SVG-only form.
Using load.php modules is also possible, but will pull down other unnecessary icons
and break if any of these modules are changed. Decided that we should manually maintain
these for the time being given there are only 3 icons.
- Several LESS files now import the variables file. I think it's useful for stories
to only import the CSS they use as this encourages us to modularise our CSS. Before these
imports were not necessary as they inherit imports from index.less. This will have no impact
on the bundle size as the LESS compiler silently discards duplicate imports
- stories/utils.js provides a useful placeholder function for generalising our hook entry
points.
Bug: T242674
Change-Id: I722e84d2fb57653a2f96142dc3e5248043261746
Use a template variable only for html attributes of the logo
Move static HTML into Navigation.mustache
Change-Id: If37015b9ce4f37e264b6f25956e4d0ca35e8cdff
Vector provides two vector specific hooks:
- VectorAfterToolbox
- VectorBeforeToolbox
Instead of supporting Vector specific stuff we should aim
for a nice global skin api and allow other 3rd party to
extend MediaWiki functionality no matter which skin is currently
on.
Bug: T240062
Change-Id: I245f1316e79f814ba04f4e0a0223d4f0596cf39e
This will allow us to render in Storybook without having issues
with unclosed tags. it also mirrors how html-headelement works
(obscuring the opening of the body and html tags)
Bug: T240062
Bug: T242674
Change-Id: I216a920c68bf3da9de55a75fc53451c68c9cc753
A Portal (or portlet) in the Vector context is a section of the
sidebar menu (e.g. Tools, Languges etc.).
The hook that places content between the portals (or portlets?)
such as `SkinTemplateToolboxEnd` and `otherlanguages` has been
enclosed inside of an output buffer so that any content it
produces can be passed to a template.
Bug: T239248, T240062
Change-Id: I882db161e5462cf88aa48c9cfd91448eb97a4a77
Extracts a new VectorMenu mustache component from VectorTemplate.
VectorMenu is the "more" menu that appears at small widths instead of the
Read/Edit/View History menu near the top of the Vector skin.
Bug: T239248, T240062
Change-Id: I41b1ec949d81303abddadb981741445572c939e3
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
PersonalMenu is the login/logout notifications etc. menu at the very
top of the Vector skin.
Bug: T239248, T240062
Change-Id: Iae224cbd838e44669a9f27e6dd303c6c3b402d41
For consistency with VectorTabs, rename SearchComponent to only imply
component. At least two word names seems like a good target (instead of
just "Search") for grepability and standard component style conventions.
Bug: T239248
Change-Id: I1e4f7270ba29c2f35f08e92f8a28cd8a2ec8fe87
The tab attributes were mistakenly being echoed instead of passed by
value. E.g., http://localhost:8181/wiki/A?uselang=he.
Bug: T239248
Change-Id: Ia0af96df717948743a2d66f8432b47e6f8a9c8f1
Creates a new VectorTabs.mustache component and uses it to render
the namespace & view tabs.
Bug: T239248
Change-Id: I859e4e95a2a12470f66564db547679c9f0a16727
Extract SearchComponent.mustache from VectorTemplate. The "search"
message is now escaped by the template parser. As is, htmlspecialchars()
for "wgScript".
A later patch will change each component renderer function as well as
renderNavigation() to return a string.
Change-Id: I3084b7e0ef73d320c85ee780c9eff13ecea92906
Move template logic outside of the template and into variables. This
will allow a future refactor to move the template to a separate
template-only file.
Change-Id: I91c2df11106cf09428a890a2d96ecbddb18ef0fe
Cut up renderNavigation() into several discrete methods. Smaller methods
are easier to read and reason about.
Change-Id: I4e55d858ab1745fb2baddef02f7046ec842eb149
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
Regression from 74b9803d9a, caused by a bug in LightNCandy which
caused {{foo}} to render "0", and {{#foo}} to pass as true with "0", but then
in {{#foo}}<b>{{foo}}</b>{{/foo} render as empty string producing "<b></b>".
In other words, the conditional is passing and the inner block is executed,
but the placeholder is mistakenly converting "0" => null => "" (empty string),
causing the <h1> to render but without any text in it.
Work around this bug by simply removing the conditional. Several other skins
already don't have this conditional and it's unclear why or in what
situation MediaWiki would send OutputPage to SkinTemplate without a title.
I think it would make sense in such rare case to still have a consistent
layout for extensions and gadgets to interact with and not omit the H1
element, but render it with the value that OutputPage gave it, even if it
is the empty string.
Bug: T219864
Change-Id: I6e04b512d2fe2e949ff5385cb38ceebe392fb255
This reverts commit a3ca2c3e16.
Reason for revert: This requires wider discussion before moving
forward, and a more complete implementation even once we do have
consensus.
No associated task exists on which to view or continue this
discussion: linked task briefly mentions Mustache in general as an
option as part of a much wider topic, but doesn't concern this
specifically.
Issues that should be discussed include:
* What the intent even is here: is this for one skin only? Is this
the intended path forward for all of them? Depending on which, we
have other issues: for the former case, that it is quite
unhelpful in terms of maintenance and further development having
more random code diversity out there, especially in this
half-completed state; or if it is indeed intended for all of them,
that an RfC is needed before anything is merged, as that is a very
significant change.
* That using Mustache in MediaWiki does add (usually minor)
performance overhead; we need to clearly establish in the task that
this is indeed worth it here.
Change-Id: I0bafa55b554aa8a38553e20c75859ec5eec2c062
The VectorTemplate class extends BaseTemplate, which has defined
this method since MW 1.25. The Vector skin master branch supports
MW 1.29+ only.
Change-Id: I83c6add9e8c02df028ca5905934e7d367dbe2209
Xml::expandAttributes() outputs a space at the beginning before it
outputs the attributs.
This change avoids a double space between the attributes.
Before this change the HTML contains:
<a class="mw-wiki-logo" href="..." title="...">
After this change the HTML contains:
<a class="mw-wiki-logo" href="..." title="...">
Change-Id: I486d26bd56a4410766f40b78466c2f3559f3a1ff
Before this change the HTML contains:
<div id="p-personal" role="navigation" class="" ...>
or
<div id="p-personal" role="navigation" class=" emptyPortlet" ...>
After this change the HTML contains:
<div id="p-personal" role="navigation" ...>
or
<div id="p-personal" role="navigation" class="emptyPortlet" ...>
Change-Id: Ic686b958940afc958693d0031ac31e5f783960a9
Unordered lists can be absolute positioned down to IE 7.
Outdated selector `.vectorMenu ul` remains for a release cycle
until HTML cache is renewed.
Bug: T209558
Change-Id: Id18ca9a8d705572b1f7e17920ef52b80e9aec373
* Improve their accessibility by giving both links
a full label "Jump to x" and "Jump to y" instead
of "Jump to: ", "x", "y".
This also makes things much better for localisation, for which
we generally discourage use of concatenation.
* Use pure CSS for the toggling of the visibility on focus,
instead of relying on JavaScript. Especially given the
JS comes form core's 'jquery.mw-jump' module, which is
considered technical debt per T195256. Alternatively,
that could be copied to vector.js, but pure CSS
is possible, so why not.
* Use plain <a> links in the HTML instead of wrapped in a <div>.
This solves the long-standing problem whereby the margin
between #contentSub and #mw-content-text had to be awkwardly
negated and overridden in core and on various to make sure that
the wrapper itself would become visible as needed, in a way that
has margin around this. This whole problem doesn't apply when
simply using inline links that aren't part of the regular flow
with .mixin-screen-reader-text. On focus, the individually
focussed link appears in regular flow, without the need for
any custom styles.
* This uses :not(:focus) to naturally make it render in the default
way on focus, and visibibly hidden/clipped otherwise.
This is supported in IE9+ and Android 2+.
There is a way to make it work with CSS2 for IE7-8, by applying
the mixin to '.mw-jump-link' only and then undoing all of
'position', 'width', 'height', 'clip', and 'margin' on :focus.
But I'm not sure that's worth it here. The fallback in IE7-8
for not supporting ":not(:focus)" is that the accessibility
link is simply visible always, which seems like a good fallback
for accessibility, and doesn't hurt anything.
Bug: T195256
Change-Id: Icaadb290f692b3617688d32cbb66dfb007f1c82c
