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
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
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
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
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
