Commit graph

70 commits

Author SHA1 Message Date
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
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 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
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
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
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
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 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 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
Stephen Niedzielski ac069fbec1 [dev] Consolidate ResourceLoader LESS style files
Move styles.less to index.less and import print.less from it. This keeps
the reasoning about styles constrained to LESS instead of spread out
over LESS _and_ ResourceLoader. The former is preferable since LESS is
more standardized than ResourceLoader.

The approach of moving styles.less to index.less and then referencing
print.less was chosen with the intent that it'd be easier to assume
styles are screen styles unless a media query says otherwise.

This patch also makes the variables import common among print and screen
styles.

Bug: T246419
Change-Id: I981d0937aaacb7cba082c337f98c90e90b46b340
2020-03-10 15:29:15 -06:00
Nicholas Ray 892eb489e6 Move watchstar import out of VectorTabs.less and into screen.less
* VectorTabs.stories.js and Navigation.stories.js were updated to
reflect this change.

Bug: T243281
Change-Id: I96a3b9b2c9a8d799a5835de1f296bc1a779803ee
2020-03-10 10:04:46 -06:00
Nicholas Ray e8a403e342 Rename Storybook Files to Reflect Their Respective Component
* Following up on the work from
Idf90ee2a0f1c1d08a31cf50099c0bebc7b67e619, this commit renames the
storybook files/storybook names to their respective component name.

e.g. personalNavigation.stories.js => PersonalMenu.stories.js

Bug: T243281
Change-Id: I68054663c5a597f90a826b6f75bf399382dca609
2020-03-03 18:32:55 +00:00
Nicholas Ray 9823538683 Isolate Vector Styles to their Respective Component
This will help with the encapsulation/reusability of each component.

* Stylesheets were renamed to reflect their respective component name
(e.g. search.less became SearchBox.less)

* Styles were isolated to each component:

* navigation.less now only contains classes that are relevant to
Navigation.mustache.
  * personalNavigation.less, search.less, and tabs.less
    imports were removed and made first-class styles.
  * several selectors were moved into common.less
  * #p-logo was moved into sidebar

* tabs.less was renamed to VectorTabs.less and styles specific to
VectorMenu.less were put into VectorMenu.less

* Storybook was updated to reflect changes

Bug: T243281
Change-Id: Idf90ee2a0f1c1d08a31cf50099c0bebc7b67e619
2020-03-03 18:20:19 +00:00
jdlrobson cdd5ebd74d Use template partials rather than HTML strings
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
2020-03-03 09:28:52 -08:00
jdlrobson 698752e38a [Dev] processTemplate used in one place
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
2020-02-26 12:28:40 -08:00
jdlrobson ea2bcd44f2 Add storybook to Vector
- 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
2020-01-31 16:59:15 +08:00