Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Post Schedule: show post events #29716

Merged
merged 29 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
62b3e05
post-schedule: collect and pass calendar events
retrofox Mar 9, 2021
3352cc2
post-schedule: re-write component using function
retrofox Mar 9, 2021
c522705
date-time: remove onMonthPreviewed() and highlight
retrofox Mar 9, 2021
ce24e97
post-schedule: update current month
retrofox Mar 9, 2021
58047af
date-time: render day events using own component
retrofox Mar 10, 2021
378ca18
date-time: styling when not selected day
retrofox Mar 10, 2021
0908be5
fixing eslint issues
retrofox Mar 10, 2021
3db4fb6
date-time: tweak calendar events style
retrofox Mar 10, 2021
ce1e4ed
post-schedule: exlude current post from evenys
retrofox Mar 10, 2021
d43f042
date-time: decouple onMonthChange with keepFocusInside
retrofox Mar 10, 2021
8a71f85
date-time: check events before to pick events day
retrofox Mar 12, 2021
4e3471e
fix rebasing issues
retrofox Mar 15, 2021
415f543
date-time: events replace circles by dots
retrofox Mar 15, 2021
51e49d2
fix rebasing bug
retrofox Mar 15, 2021
efed560
post-schedule: split up processing post events
retrofox Mar 16, 2021
7db090c
date-time: revert prop name change
retrofox Mar 16, 2021
bd196cf
date-time: add screen reader support
retrofox Mar 16, 2021
07d1f8e
date-time: update content for screen reader
retrofox Mar 17, 2021
576ec79
date-time: fix eslint error
retrofox Mar 17, 2021
79567d3
post-schedule: get events by post type
retrofox Mar 17, 2021
967bff1
date-time: rephrase events number for readers
retrofox Mar 17, 2021
0ced50a
date-time: add events to aria label
retrofox Mar 22, 2021
3645a7a
date-time: fix lint error. tidy.
retrofox Apr 6, 2021
212bb21
date-time: set aria-label value
retrofox Apr 6, 2021
3a9bc18
date-time: update e2e test
retrofox Apr 6, 2021
fa33d94
date-time: clean comment
retrofox Apr 7, 2021
f677f37
date-time: join day-with-events string to improve its translation
retrofox Apr 7, 2021
153fac3
date-time: remove ref as hook dependency
retrofox Apr 8, 2021
2fb0991
post-scheudle: use title.rendered prop for events
retrofox Apr 8, 2021
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
93 changes: 76 additions & 17 deletions packages/components/src/date-time/date.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,88 @@
* External dependencies
*/
import moment from 'moment';
import classnames from 'classnames';

// react-dates doesn't tree-shake correctly, so we import from the individual
// component here, to avoid including too much of the library
import DayPickerSingleDateController from 'react-dates/lib/components/DayPickerSingleDateController';
retrofox marked this conversation as resolved.
Show resolved Hide resolved

/**
* WordPress dependencies
*/
import { Component, createRef } from '@wordpress/element';
import { isRTL } from '@wordpress/i18n';
import { Component, createRef, useEffect, useRef } from '@wordpress/element';
import { isRTL, _n, sprintf } from '@wordpress/i18n';

/**
* Module Constants
*/
const TIMEZONELESS_FORMAT = 'YYYY-MM-DDTHH:mm:ss';
const ARIAL_LABEL_TIME_FORMAT = 'dddd, LL';

function DatePickerDay( { day, events = [] } ) {
const ref = useRef();

/*
* a11y hack to make the `There is/are n events` string
* available speaking for readers,
* re-defining the aria-label attribute.
* This attribute is handled by the react-dates component.
*/
useEffect( () => {
// Bail when no parent node.
if ( ! ref?.current?.parentNode ) {
retrofox marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const { parentNode } = ref.current;
const dayAriaLabel = moment( day ).format( ARIAL_LABEL_TIME_FORMAT );
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I noticed and better left for a follow up PR:

This is one of the few usages of moment in the project. It looks like we might have some options using @wordpress/date or even native browser support if that eventually falls into WP support ranges:


if ( ! events.length ) {
// Set aria-label without event description.
parentNode.setAttribute( 'aria-label', dayAriaLabel );
return;
}

const dayWithEventsDescription = sprintf(
// translators: 1: Calendar day format, 2: Calendar event number.
_n(
'%1$s. There is %2$d event.',
'%1$s. There are %2$d events.',
events.length
),
dayAriaLabel,
events.length
);

parentNode.setAttribute( 'aria-label', dayWithEventsDescription );
}, [ ref, events.length ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand why we're accessing and modifying attributes using DOM APIs here. I'm pretty sure you can get React warnings when doing things like that to Dom elements controlled by React.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, using a "ref" as dependency is useless, a ref never changes its instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really understand why we're accessing and modifying attributes using DOM APIs here.

Because the component API doesn't support extending the aria-label attribute in a super flexible way. It's nice when setting the date format, which is great. We can use, and we do, the dayAriaLabelFormat prop:

<DayPickerSingleDateController
    dayAriaLabelFormat="dd"
/>

It generates the following markup (Saturday day):

<td aria-label"Sa" ...>

Even is possible to set the property using arbitrary text, something like...

<DayPickerSingleDateController
    dayAriaLabelFormat="dd [🦊 retrofox]"
/>

Screen Shot 2021-04-08 at 7 23 42 AM

However, it isn't enough for our requirements, since what we'd like to achieve is setting a custom aria-label for each rendering day. Something like the following:

<DayPickerSingleDateController
	dayAriaLabelFormat={ ( day ) => {
		const dayAriaLabel = moment( day ).format( ARIAL_LABEL_TIME_FORMAT );
		const events = this.getEventsPerDay( day );

		return sprintf(
			// translators: 1: Calendar day format, 2: Calendar event number.
			_n(
				'%1$s. There is %2$d event.',
				'%1$s. There are %2$d events.',
				events.length
			),
			dayAriaLabel,
			events.length
		);
	} }

	// ....
/>

However, dayAriaLabelFormat only accepts a string. :'(

I'm pretty sure you can get React warnings when doing things like that to Dom elements controlled by React.

I think everybody agrees with that. The approach is quite far away to be ideal, but it works and it shouldn't hurt in the sense the app shouldn't get broken. In the worse cases, it should perform any change in the markup.
Our issue is not being able to extend the component as we wish, and AFAIK, DOM manipulation is the only solution without changing the react-dates component, something that we researched a few months ago.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do expect this to break often though, React might restrict us from doing that. Do we have warnings now in the console because of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All warnings that I see belong to the react-dates component. They already are in the trunk branch. I don't see any about the DOM manipulation process.

image

image


return (
<div
ref={ ref }
className={ classnames( 'components-datetime__date__day', {
'has-events': events?.length,
} ) }
>
{ day.format( 'D' ) }
</div>
);
}

class DatePicker extends Component {
constructor() {
super( ...arguments );

this.onChangeMoment = this.onChangeMoment.bind( this );
this.nodeRef = createRef();
this.keepFocusInside = this.keepFocusInside.bind( this );
this.isDayHighlighted = this.isDayHighlighted.bind( this );
this.onMonthPreviewedHandler = this.onMonthPreviewedHandler.bind(
this
);
}

onMonthPreviewedHandler( newMonthDate ) {
this.props?.onMonthPreviewed( newMonthDate.toISOString() );
this.keepFocusInside();
}

/*
Expand Down Expand Up @@ -89,19 +148,13 @@ class DatePicker extends Component {
return currentDate ? moment( currentDate ) : moment();
}

// todo change reference to `isDayHighlighted` every time, `events` prop change
isDayHighlighted( date ) {
// Do not highlight when no events.
getEventsPerDay( day ) {
if ( ! this.props.events?.length ) {
return false;
}
if ( this.props.onMonthPreviewed ) {
this.props.onMonthPreviewed( date.toDate() );
return [];
}

// Compare date against highlighted events.
return this.props.events.some( ( highlighted ) =>
date.isSame( highlighted.date, 'day' )
return this.props.events.filter( ( eventDay ) =>
day.isSame( eventDay.date, 'day' )
);
}

Expand All @@ -126,13 +179,19 @@ class DatePicker extends Component {
onDateChange={ this.onChangeMoment }
transitionDuration={ 0 }
weekDayFormat="ddd"
dayAriaLabelFormat={ ARIAL_LABEL_TIME_FORMAT }
isRTL={ isRTL() }
isOutsideRange={ ( date ) => {
return isInvalidDate && isInvalidDate( date.toDate() );
} }
isDayHighlighted={ this.isDayHighlighted }
onPrevMonthClick={ this.keepFocusInside }
onNextMonthClick={ this.keepFocusInside }
onPrevMonthClick={ this.onMonthPreviewedHandler }
onNextMonthClick={ this.onMonthPreviewedHandler }
renderDayContents={ ( day ) => (
<DatePickerDay
day={ day }
events={ this.getEventsPerDay( day ) }
/>
) }
/>
</div>
);
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/date-time/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// Needed to initialise the default datepicker styles.
// See: https://github.com/airbnb/react-dates#initialize
import 'react-dates/initialize';
import { noop } from 'lodash';
Copy link
Contributor

Choose a reason for hiding this comment

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

For other reviewers, are we trying to reduce lodash usage in the project?


/**
* WordPress dependencies
Expand All @@ -25,7 +26,7 @@ function DateTimePicker(
currentDate,
is12Hour,
isInvalidDate,
onMonthPreviewed,
onMonthPreviewed = noop,
onChange,
events,
},
Expand All @@ -52,8 +53,8 @@ function DateTimePicker(
currentDate={ currentDate }
onChange={ onChange }
isInvalidDate={ isInvalidDate }
onMonthPreviewed={ onMonthPreviewed }
events={ events }
onMonthPreviewed={ onMonthPreviewed }
/>
</>
) }
Expand Down
25 changes: 25 additions & 0 deletions packages/components/src/date-time/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,31 @@
}
}

.components-datetime__date .CalendarDay .components-datetime__date__day {
height: 100%;
display: flex;
justify-content: center;
align-content: center;
flex-direction: column;
position: relative;

&.has-events::before {
content: " ";
width: 4px;
height: 4px;
border-radius: 2px;
position: absolute;
left: 50%;
margin-left: -2px;
bottom: 0;
background-color: $white;
}
}

.components-datetime__date .CalendarDay:not(.CalendarDay__selected) .components-datetime__date__day.has-events::before {
background: var(--wp-admin-theme-color);
}

.components-datetime__time {
padding-bottom: $grid-unit-20;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe( 'Post visibility', () => {
);
await (
await page.$x(
'//td[contains(concat(" ", @class, " "), " CalendarDay ")][text() = "15"]'
'//td[contains(concat(" ", @class, " "), " CalendarDay ")]/div[contains(concat(" ", @class, " "), " components-datetime__date__day ")][text() = "15"]'
)
)[ 0 ].click();

Expand Down
81 changes: 61 additions & 20 deletions packages/editor/src/components/post-schedule/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,68 @@
* WordPress dependencies
*/
import { __experimentalGetSettings } from '@wordpress/date';
import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { useDispatch, useSelect } from '@wordpress/data';
import { DateTimePicker } from '@wordpress/components';
import { useRef } from '@wordpress/element';
import { useRef, useState, useMemo } from '@wordpress/element';
import { store as coreStore } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import { store as editorStore } from '../../store';

function getDayOfTheMonth( date = new Date(), firstDay = true ) {
const d = new Date( date );
return new Date(
d.getFullYear(),
d.getMonth() + ( firstDay ? 0 : 1 ),
firstDay ? 1 : 0
).toISOString();
}
retrofox marked this conversation as resolved.
Show resolved Hide resolved

export default function PostSchedule() {
const { postDate, postType } = useSelect(
( select ) => ( {
postDate: select( editorStore ).getEditedPostAttribute( 'date' ),
postType: select( editorStore ).getCurrentPostType(),
} ),
[]
);

const { editPost } = useDispatch( editorStore );
const onUpdateDate = ( date ) => editPost( { date } );

const [ previewedMonth, setPreviewedMonth ] = useState(
getDayOfTheMonth( postDate )
);

// Pick up published and schduled site posts.
const eventsByPostType = useSelect(
( select ) =>
select( coreStore ).getEntityRecords( 'postType', postType, {
status: 'publish,future',
after: getDayOfTheMonth( previewedMonth ),
before: getDayOfTheMonth( previewedMonth, false ),
exclude: [ select( editorStore ).getCurrentPostId() ],
} ),
[ previewedMonth, postType ]
);

const events = useMemo(
() =>
( eventsByPostType || [] ).map(
( { title, type, date: eventDate } ) => ( {
title: title?.raw,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do folks know if we need the raw or rendered value 🤔

Copy link
Contributor Author

@retrofox retrofox Apr 7, 2021

Choose a reason for hiding this comment

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

I wasn't sure either and since it isn't going to be consumed on this approach, I preferred using the raw version of the title. However, if we'd like to implement the events tooltip we may like to go for the rendered version to format the event list:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR #29737

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should be the rendered title, the raw is used when editing only AFAIK

type,
date: new Date( eventDate ),
} )
),
[ eventsByPostType ]
);

export function PostSchedule( { date, onUpdateDate } ) {
const ref = useRef();
const settings = __experimentalGetSettings();

// To know if the current timezone is a 12 hour time with look for "a" in the time format
// We also make sure this a is not escaped by a "/"
const is12HourTime = /a(?!\\)/i.test(
Expand All @@ -30,24 +84,11 @@ export function PostSchedule( { date, onUpdateDate } ) {
return (
<DateTimePicker
ref={ ref }
currentDate={ date }
currentDate={ postDate }
onChange={ onChange }
is12Hour={ is12HourTime }
events={ events }
onMonthPreviewed={ setPreviewedMonth }
/>
);
}

export default compose( [
withSelect( ( select ) => {
return {
date: select( 'core/editor' ).getEditedPostAttribute( 'date' ),
};
} ),
withDispatch( ( dispatch ) => {
return {
onUpdateDate( date ) {
dispatch( 'core/editor' ).editPost( { date } );
},
};
} ),
] )( PostSchedule );