Commit Graph

119 Commits

Author SHA1 Message Date
jdlrobson 9cd47c5339 Refactor: VectorMenu merged in to Menu
Bug: T249372
Change-Id: Ifaf78b507c12aa251228213c89751cbb4d111d9a
2020-05-06 16:09:22 -07:00
jdlrobson a43e79c1d3 Refactor: Merge VectorTabs into Menu
Bug: T249372
Change-Id: Ib6ae191b31564dc23a3b1d6aedf48cbaaad006af
2020-05-06 10:23:58 -07:00
jdlrobson a3bb097cf8 Refactor: Generalise personal menu
the PersonalMenu should be generalised. In future we will use it as
the template for all menus

Bug: T249372
Change-Id: Id1c43d2e9eefef1d7aec45f0137e27f10ad935df
2020-05-05 17:34:44 -07:00
jenkins-bot 4086a4c824 Merge "Refactor: Remove indirection (where alternatives exist)" 2020-05-05 20:34:03 +00:00
jdlrobson 7efe7c3468 Refactor: Remove indirection (where alternatives exist)
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
2020-05-05 13:07:14 -07:00
Nicholas Ray 91c25bc253 Add title attribute to opt-out link
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
2020-05-05 09:52:20 -06:00
jdlrobson 97121a228f [Minor] Favor msg and getConfig over getMsg and ->config
Change-Id: I69ff5b8fc51292f745e0aa496b8129053c9d2f23
2020-05-01 19:11:58 +00:00
jenkins-bot 48b680744d Merge "Use consistent function names to SkinVector" 2020-04-30 22:16:06 +00:00
jenkins-bot 337244ebdf Merge "[modern] A new version of Vector with a new logo" 2020-04-30 21:47:44 +00:00
jdlrobson 6a9ee465bc [modern] A new version of Vector with a new logo
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
2020-04-30 14:11:54 -07:00
jenkins-bot ec6307e669 Merge "[fix] "Existing account only" skin version config" 2020-04-29 18:55:30 +00:00
Stephen Niedzielski 709772fa12 [fix] "Existing account only" skin version config
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
2020-04-29 18:36:03 +00:00
Sam Smith d4c2e2b441 [Hygiene] featureManager: ComplexRequirement -> Requirement
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
2020-04-29 15:26:50 +01:00
jdlrobson d9f4a2b041 Use consistent function names to SkinVector
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
2020-04-28 15:32:06 -07:00
jenkins-bot db29a88ee0 Merge "Add buildSidebar method" 2020-04-28 20:49:31 +00:00
jenkins-bot 1a62d2d264 Merge "[cleanup] Drop empty `class` attribute from `#footer > ul` elements" 2020-04-28 18:41:40 +00:00
jdlrobson df681e031d Add buildSidebar method
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
2020-04-28 10:44:39 -07:00
AronDemian f6cbbbfd95 [modern] Add Header element and styles
- 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
2020-04-28 18:15:39 +02:00
AronDemian 487eda02f4 [cleanup] Drop empty `class` attribute from `#footer > ul` elements
Change-Id: Ice0241b054cbc296e9f0adf1d2920023316c8ca0
2020-04-28 02:05:30 +00:00
jenkins-bot b2dee16b63 Merge "Add `.mw-footer` CSS class & replace `#footer` selector with it" 2020-04-27 22:49:47 +00:00
AronDemian c2e9a7b366 Add `.mw-footer` CSS class & replace `#footer` selector with it
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
2020-04-27 14:55:28 -07:00
jdlrobson b42493a476 Refactor: DRY up menu creation!
Bug: T249372
Change-Id: I098e6921e8f7ef65dacacf09b9c25f70c945e58e
2020-04-27 13:36:39 -07:00
jdlrobson 0b8693ccec Refactor: Make VectorTabs template data conform with MenuDefinition
menu-id -> id
menu-label-id -> label-id
msg-label -> label
empty-portlet -> class

Bug: T249372
Change-Id: I15fd88269b3d111ff47e64f3669575afaeea9f97
2020-04-22 05:18:44 +00:00
jdlrobson 286b07b20a Refactor: Make VectorMenu template data conform with MenuDefinition
menu-id -> id
menu-label-id -> label-id
msg-label -> label
empty-portlet -> class

Bug: T249372
Change-Id: I50bf2424f9077ac987e03a1e552577bf7c48b3bb
2020-04-22 05:18:29 +00:00
Sam Smith 37fc80d562 [Hygiene] featureManager: Move SimpleRequirement...
... to the Vector\FeatureManagement\Requirements namespace.

Change-Id: I205eff9d257c040874fe31e5eda33b4e8cb229cc
2020-04-21 18:22:07 +01:00
Sam Smith cee5ee0003 Make legacy mode a feature
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
2020-04-21 10:57:11 -06:00
Stephen Niedzielski 3b906fdd66 [dev] Fix indent in VectorTemplate data
Jon spotted this in Ic9d54de7e19ef8d5dfd703d95a45b78c0aaf791a.

Bug: T246419
Change-Id: Ie760610898c1754ac0640e8077faa6ec79da66c1
2020-04-17 14:23:56 -06:00
Volker E c3f9194144 Fix Special:Preferences sub section link
Now links to nested sub sections are possible with
I33318b3fb73a27ce8013 in.

Bug: T248644
Change-Id: I92d0910191dcab0a7fe43ba6976c6773384688a7
2020-04-17 00:08:42 +00:00
jdlrobson 703903daac [legacy] Split sidebar code and mark layout as legacy in preparation for new layout
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
2020-04-16 11:29:03 -07:00
jdlrobson 4d91d52dfc Add a special class to identify the first portal
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
2020-04-15 23:39:33 +00:00
jdlrobson 63ee9450b7 Refactor: Standardize menu data (DRY!)
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
2020-04-13 16:11:40 -07:00
Stephen Niedzielski 16670834d7 [dev] [Mustache] replace spaces with tabs
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
2020-04-08 15:40:56 -06:00
VolkerE 087808ece6 Revert "Move indicators underneath `#firstHeading` in DOM"
This reverts commit f400fc57c3.

Bug: T248761
Change-Id: I3a4f1202d5e1cca9f82d1735cc498049fa25613c
2020-04-08 13:43:10 +00:00
Volker E f400fc57c3 Move indicators underneath `#firstHeading` in DOM
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
2020-04-06 17:06:31 -07:00
Stephen Niedzielski d1072d0fdf [dev] add skin version query parameter override
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
2020-04-02 16:22:26 -06:00
AronDemian f2fad08fb7 Define the legacy and modern root template names in the same location (improve locality)
Follow-up to I580fe55ca6aeb35dae8afee41e87172a5a729c7b

Change-Id: Iea8be07c7a86c5ca258ffdfa07fa52713cb1d6d5
2020-04-02 01:55:55 +02:00
jdlrobson c8001f91b1 Split legacy and modern experience on template level
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
2020-03-31 14:35:00 -07:00
jdlrobson 872519ab94 Drop the Navigation component
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
2020-03-30 15:26:34 -07:00
jenkins-bot d2b19192c7 Merge "Add opt-out link to Sidebar for Vector/Logged-in Users Without Abstractions" 2020-03-27 00:36:29 +00:00
Nicholas Ray ec382a8c86 Add opt-out link to Sidebar for Vector/Logged-in Users Without Abstractions
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
2020-03-26 17:39:47 -06:00
Stephen Niedzielski 6b6bed86ed [docs] [dev] [PHP] [FeatureManager] revise docs and add DynamicConfigRequirement test
Address some feedback from I7a2cdc2dfdf20d78e4548f07cf53994563b234b3:

- Miscellaneous documentation improvements.
- Add a false case test to `DynamicConfigRequirement->isMet()`.

Bug: T244481
Change-Id: Ic5637f42da755f871c5a6d545e14effd3ac8c670
2020-03-26 20:39:40 +00:00
jdlrobson 4091a6729f Document setupTemplate method and mark Vector as @final
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
2020-03-25 18:04:16 +00:00
jenkins-bot f855c3948e Merge "featureManager: Add DynamicConfigRequirement requirement" 2020-03-25 15:20:47 +00:00
Nicholas Ray 9964e56156 Mustache Documentation: Cleanup/Clarify Mustache template params
* 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
2020-03-18 14:47:31 -07:00
Sam Smith 84226b41b6 featureManager: Add DynamicConfigRequirement requirement
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
2020-03-18 10:10:42 +00:00
Stephen Niedzielski bd7bd75569 [JavaScript] Validate types
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
2020-03-16 09:10:08 -06:00
jdlrobson 9a3a3d3c96 Ship different ResourceLoader module for different versions
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
2020-03-12 13:36:52 -07:00
jdlrobson 59747a58c8 Separate first portal in sidebar from rest
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
2020-03-11 23:34:11 +00:00
jenkins-bot c3bbe2acb3 Merge "docs: Improve VectorTemplate Mustache param docs" 2020-03-11 20:25:56 +00:00
Sam Smith d14caf2f11 featureManager: Add Requirement interface
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
2020-03-09 11:06:26 +00:00