- 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
