Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/volto/news/6732.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace `moment.js` with native `Intl` formatting in `Comments` and `EventDatesInfo` components. @avoinea
24 changes: 17 additions & 7 deletions packages/volto/src/components/theme/Comments/Comments.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ import PropTypes from 'prop-types';
import { defineMessages, FormattedMessage, useIntl } from 'react-intl';
import { createPortal } from 'react-dom';
import { useDispatch, useSelector, shallowEqual } from 'react-redux';
import { compose } from 'redux';

import { Button, Comment, Container, Icon } from 'semantic-ui-react';

import { injectLazyLibs } from '@plone/volto/helpers/Loadable/Loadable';
import {
formatDate,
formatRelativeDate,
} from '@plone/volto/helpers/Utils/Date';
import {
addComment,
deleteComment,
Expand Down Expand Up @@ -200,8 +203,6 @@ const Comments = (props) => {
return allCommentsWithCildren;
};

const moment = props.moment.default;

const allCommentsWithCildren = useMemo(
() => addRepliesAsChildrenToComments(items),
[items],
Expand All @@ -223,8 +224,17 @@ const Comments = (props) => {
<Comment.Metadata>
<span>
{' '}
<span title={moment(comment.creation_date).format('LLLL')}>
{moment(comment.creation_date).fromNow()}
<span
title={formatDate({
date: comment.creation_date,
format: 'LLLL',
locale: intl.locale,
})}
>
{formatRelativeDate({
date: comment.creation_date,
locale: intl.locale,
})}
</span>
</span>
</Comment.Metadata>
Expand Down Expand Up @@ -375,4 +385,4 @@ Comments.propTypes = {
pathname: PropTypes.string.isRequired,
};

export default compose(injectLazyLibs(['moment']))(Comments);
export default Comments;
25 changes: 10 additions & 15 deletions packages/volto/src/components/theme/Comments/Comments.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,18 @@ vi.mock('@plone/volto/components/theme/Comments/CommentEditModal', () => ({

const mockStore = configureStore();

vi.mock('moment', () => ({
default: vi.fn(() => ({
format: vi.fn(() => 'Sunday, April 23, 2017 3:38 AM'),
fromNow: vi.fn(() => 'a few seconds ago'),
})),
}));

vi.mock('@plone/volto/helpers/Loadable/Loadable');
vi.mock('@plone/volto/components/manage/Form');
beforeAll(async () => {
const { __setLoadables } = await import(
'@plone/volto/helpers/Loadable/Loadable'
);
await __setLoadables();
});

describe('Comments', () => {
beforeEach(() => {
// Freeze time so formatRelativeDate produces deterministic output
vi.useFakeTimers({ now: new Date('2017-04-23T03:38:04Z') });
});

afterEach(() => {
vi.useRealTimers();
});

it('renders a comments component', () => {
const store = mockStore({
comments: {
Expand All @@ -37,7 +32,7 @@ describe('Comments', () => {
'@id': 'someurl',
comment_id: '1614094601171408',
author_name: 'admin',
creation_date: '2017-11-06T19:36:01',
creation_date: '2017-04-23T03:38:00Z',
text: { data: 'Some comment' },
is_deletable: true,
is_editable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ exports[`Comments > renders a comments component 1`] = `
<span>

<span
title="Sunday, April 23, 2017 3:38 AM"
title="Sunday, April 23, 2017 at 3:38 AM"
>
a few seconds ago
4 seconds ago
</span>
</span>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,45 @@ exports[`renders event details component with all props 1`] = `
>
When
</div>
<p
className="event-when"
>
<span
className="start"
>
<span
className="start-date"
>
Jun 23, 2019
</span>
<span>

</span>
<span
className="start-time"
>
3:20 PM
</span>
</span>
 to 
<span
className="end"
>
<span
className="end-date"
>
Jun 24, 2019
</span>
<span>

</span>
<span
className="end-time"
>
3:20 PM
</span>
</span>
</p>
<div
className="ui dividing sub header"
>
Expand Down Expand Up @@ -146,6 +185,27 @@ exports[`renders event details component with only required props 1`] = `
>
When
</div>
<p
className="event-when same-day"
>
<span
className="start-date"
>
Jun 23, 2019
</span>
 from 
<span
className="start-time"
>
3:20 PM
</span>
 to 
<span
className="end-time"
>
4:20 PM
</span>
</p>
<div
className="download-event"
>
Expand Down Expand Up @@ -188,6 +248,27 @@ exports[`renders event details component without links to api in the text 1`] =
>
When
</div>
<p
className="event-when same-day"
>
<span
className="start-date"
>
Jun 23, 2019
</span>
 from 
<span
className="start-time"
>
3:20 PM
</span>
 to 
<span
className="end-time"
>
4:20 PM
</span>
</p>
<div
className="download-event"
>
Expand Down
56 changes: 35 additions & 21 deletions packages/volto/src/components/theme/View/EventDatesInfo.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@ import PropTypes from 'prop-types';
import { List } from 'semantic-ui-react';
import cx from 'classnames';

import { toBackendLang } from '@plone/volto/helpers/Utils/Utils';
import { injectLazyLibs } from '@plone/volto/helpers/Loadable/Loadable';
import { useSelector } from 'react-redux';
import { formatDate } from '@plone/volto/helpers/Utils/Date';

/**
* @deprecated Use the native Date API directly. Will be removed in Volto 20.
*/
export const datesForDisplay = (start, end, moment) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have this moment here. Will it be removed in the future? Should we note this down somewhere to remember to remove it? In an issue?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wesleybl I think @avoinea is still working on it this week.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wesleybl I deprecated datesForDisplay as it's not used in Volto Core anymore but maybe some add-on is using it. Thus we can safely remove it in the next major release, Volto 20.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avoinea Is this enough to remind us that removal is necessary? Wouldn't it be better to open an issue?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wesleybl moment js is deprecated. See also #3753

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wesleybl @davisagli

@robgietema mentioned that if we can't completely replace moment js with vanilla JS we can replace it with https://day.js.org/.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robgietema mentioned that if we can't completely replace moment js with vanilla JS we can replace it with https://day.js.org/.

@avoinea @robgietema Could you explain that in more detail?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wesleybl Not for this PR.

This one is clean enough and backward compatible and can safely be merged.

But it seems that I am hitting a wall in the DatetimeWidget / TimeWidget.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also question why to use any moment-like library, per plone/Products.CMFPlone#4115 (comment), plone/Products.CMFPlone#4115 (comment), and https://momentjs.com/docs/#/-project-status/. It doesn't make sense to me to force confusion unto the end user, as we experience on every ploneconf.org website that used it. Specifically, times of events would display in the user's timezone, not in that of the location.

Sorry to belabor the point, but I have a keen interest in all things date and time related.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also remember we already have @internationalized/date as a dependency because of react-aria-components, it helps in some situations.
https://react-aria.adobe.com/internationalized/date/

// eslint-disable-next-line no-console
console.warn(
'datesForDisplay is deprecated and will be removed in Volto 20. Use the native Date API directly.',
);
const mStart = moment(start);
const mEnd = moment(end);
if (!mStart.isValid() || !mEnd.isValid()) {
Expand All @@ -25,17 +32,30 @@ export const datesForDisplay = (start, end, moment) => {
};
};

const When_ = ({ start, end, whole_day, open_end, moment: momentlib }) => {
const lang = useSelector((state) => state.intl.locale);

const moment = momentlib.default;
moment.locale(toBackendLang(lang));
const When_ = ({ start, end, whole_day, open_end }) => {
const locale = useSelector((state) => state.intl.locale);

const datesInfo = datesForDisplay(start, end, moment);
if (!datesInfo) {
return;
const dStart = new Date(start);
const dEnd = end != null ? new Date(end) : new Date();
if (isNaN(dStart.getTime()) || isNaN(dEnd.getTime())) {
return null;
}
// TODO I18N INTL
const sameDay =
dStart.getFullYear() === dEnd.getFullYear() &&
dStart.getMonth() === dEnd.getMonth() &&
dStart.getDate() === dEnd.getDate();
const sameTime =
sameDay &&
dStart.getHours() === dEnd.getHours() &&
dStart.getMinutes() === dEnd.getMinutes();
const datesInfo = {
sameDay,
sameTime,
startDate: formatDate({ date: dStart, format: 'll', locale }),
startTime: formatDate({ date: dStart, format: 'LT', locale }),
endDate: formatDate({ date: dEnd, format: 'll', locale }),
endTime: formatDate({ date: dEnd, format: 'LT', locale }),
};
return (
<p
className={cx('event-when', {
Expand Down Expand Up @@ -100,7 +120,7 @@ const When_ = ({ start, end, whole_day, open_end, moment: momentlib }) => {
);
};

export const When = injectLazyLibs(['moment'])(When_);
export const When = When_;

When.propTypes = {
start: PropTypes.string.isRequired,
Expand All @@ -109,13 +129,8 @@ When.propTypes = {
open_end: PropTypes.bool,
};

export const Recurrence_ = ({
recurrence,
start,
moment: momentlib,
rrule,
}) => {
const moment = momentlib.default;
export const Recurrence_ = ({ recurrence, start, rrule }) => {
const locale = useSelector((state) => state.intl.locale);
const { RRule, rrulestr } = rrule;
if (recurrence.indexOf('DTSTART') < 0) {
var dtstart = RRule.optionsToString({
Expand All @@ -129,12 +144,11 @@ export const Recurrence_ = ({
<List
items={rule
.all()
.map((date) => datesForDisplay(date, undefined, moment))
.map((date) => date.startDate)}
.map((date) => formatDate({ date, format: 'll', locale }))}
/>
);
};
export const Recurrence = injectLazyLibs(['moment', 'rrule'])(Recurrence_);
export const Recurrence = injectLazyLibs(['rrule'])(Recurrence_);

Recurrence.propTypes = {
recurrence: PropTypes.string.isRequired,
Expand Down
21 changes: 21 additions & 0 deletions packages/volto/src/components/theme/View/EventDatesInfo.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,24 @@ test('same day, not whole day, not open end', () => {
const json = component.toJSON();
expect(json).toMatchSnapshot();
});

test('returns null for invalid start date', () => {
const component = renderer.create(
<Provider store={store}>
<When start="not-a-date" end="2019-06-24T15:20:00+00:00" />
</Provider>,
);
expect(component.toJSON()).toBeNull();
});

test('uses current time when end is undefined', () => {
vi.useFakeTimers({ now: new Date('2019-06-23T15:20:00+00:00') });
const component = renderer.create(
<Provider store={store}>
<When start="2019-06-23T11:55:00+00:00" />
</Provider>,
);
const json = component.toJSON();
expect(json).toMatchSnapshot();
vi.useRealTimers();
});
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,27 @@ exports[`same day, whole day 1`] = `
</span>
</p>
`;

exports[`uses current time when end is undefined 1`] = `
<p
className="event-when same-day"
>
<span
className="start-date"
>
Jun 23, 2019
</span>
 from 
<span
className="start-time"
>
11:55 AM
</span>
 to 
<span
className="end-time"
>
3:20 PM
</span>
</p>
`;
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @deprecated Will be removed in Volto 20. Use the native Date API directly. */
export function datesForDisplay(start: any, end: any, moment: any): {
sameDay: any;
sameTime: any;
Expand All @@ -7,10 +8,9 @@ export function datesForDisplay(start: any, end: any, moment: any): {
endTime: any;
};
export const When: any;
export function Recurrence_({ recurrence, start, moment: momentlib, rrule, }: {
export function Recurrence_({ recurrence, start, rrule, }: {
recurrence: any;
start: any;
moment: any;
rrule: any;
}): import("react/jsx-runtime").JSX.Element;
export const Recurrence: any;
Loading