search

plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
246 stars 186 forks source link

Remove moment.js from plone bundle #2973

Closed agitator closed 2 years ago

agitator commented 4 years ago

Responsible Persons

Proposer: Peter Holzer @agitator Seconder: Johannes Raggam @thet

Abstract

Remove moment.js and use toLocalizedTime again.

Motivation

While moment.js looked like an easy solution to display nicely formatted dates in Plone, it added a lot of KBs (~150 with all locales and ~60 with lazy loading of locales) to the anonymous bundle.

Also while displaying dates in a more spoken way "last modified a few seconds ago" looked pretty, it wasn't very useful.

Bloating the plone-bundle with this library doesn't seem very reasonable when in terms of page loading speed....

Assumptions

While working on https://github.com/plone/plone.staticresources/pull/46 to allow a more flexible and optimized loading of bundles, we checked how often pat-moment is used in a default Plone. Turns out it's only used for document_byline.pt for anon users and in content_history.pt as in menu.pt for editors. Portlets for example use toLocalizedTime anyway.

Proposal & Implementation

Proposal ist to replace pat-moment with toLocalizeTime again an make moment.js an optional/inactive bundle by default.

I'd like to see this change in Plone 5.2 but probably is too big of a change for a minor release.

Deliverables

Already separated moment.js into it's own bundle, allowing to be or not loaded depending on the usecase. https://github.com/plone/plone.staticresources/pull/46

Risks

Some addons could rely on moment.js to display dates. Therefore the addons should ideally load the bundle on request or add it to the default or logged-in bundle.

Participants

Proposer: Peter Holzer @agitator Seconder: Johannes Raggam @thet

davilima6 commented 4 years ago

I think moment has served us well and agree it's now bloat we should get rid of, specially after the new APIs in Intl.

I think however humanized date/time formatting is very central to a CMS user experience and should definitely stay. Portlets should be fixed to display humanized dates consistently with bylines in the same page, while we provide a toggle in @@dateandtime-controlpanel (we can then add a behaviour to control this per content type). Good news is Opera, FF and Chrome are caught up on standards and implement Intl.RelativeTimeFormat:

So I'd suggest as requirement for this PLIP to maintain feature parity with existing places where humanized datetimes are used (btw I noticed pat-moment is also used in menu viewlet). Non-supporting browsers like IE, Edge or Safari could fall back to non-humanized localized dates.

To implement that maybe it would ease integrators lives, and our tests, to build a Pattern with simplified API around Intl.

davilima6 commented 4 years ago

Considering MS browsers are widely used by corporations which are a Plone focus, e.g. intranets, we may adopt a polyfill for non-supporting browsers:

Or, less ideally, a lighter specialized library like humanize-duration as fallback.

tkimnguyen commented 4 years ago

Moment-formatted dates look nice and are useful at times, but very unhelpful at other times when you need to know the exact date or time of (say) a document was modified. At the very least, a per-user and per-site option for enabling/disabling it is needed.

davilima6 commented 4 years ago

I agree completely. The way other sites (Github, Facebook, Twitter) deal with this is by adding a title which you can hover, also for screen readers and general machine understanding we should wrap the humanized string in a <time> element:

<time datetime="2018-01-03T08:45:23Z" title="3 Jan 2018, 09:45 CET">2 years ago</time>

We don't do this so it sounds like a good opportunity to review it.

ebrehault commented 4 years ago

@agitator the FWT approved this PLIP

agitator commented 4 years ago

@ebrehault Nice, I think we'll discuss the details at the Alpine City Sprint

thet commented 4 years ago

Alternatives and alternative approaches to MomentJS: https://github.com/you-dont-need/You-Dont-Need-Momentjs

jensens commented 2 years ago

Old one, but is this obsolete with Plone 6 ES6 branch?

thet commented 2 years ago

The moment js bundle is removed in Plone 6. moment.js is currently still a dependency of Patternslib and might be removed in the near- to mid future but it doesn't hurt us anymore as with the async loading mechanism we do not have this big payload anymore. See: https://github.com/Patternslib/Patterns/blob/master/src/pat/display-time/display-time.js

So moment.js handling in Patternslib is fine.

I'm closing this ticket.