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
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
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
* 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
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
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
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
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
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
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
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
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
AronDemian
Jdlrobson
jdlrobson
Stephen Niedzielski
Volker E
jenkins-bot
DannyS712
Nicholas Ray
Timo Tijhof
