Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#47798 closed enhancement (fixed)

WP Events Dashboard widget could be improved for multi-days events

Reported by: imath's profile imath Owned by: desrosj's profile desrosj
Milestone: 5.4 Priority: normal
Severity: normal Version: 4.8
Component: Administration Keywords: has-patch early commit
Focuses: ui Cc:

Description

When a WordCamp is a multi-days event, only using a starting date is problematic :

  • It doesn't inform about the WordCamp's length
  • The first day of a WordCamp might be a Contributor Day (It's actually the case that generated this ticket)
  • There is some inconsistency with the schedule on central.wordcamp.org (which displays a range of days).

I've opened a ticket about it on Meta Trac concerning the needed edits (include end_date into the JSON response) for api.wordpress.org/events (4637).

Attached to this ticket you'll find the patch I suggest in order to use, just like the WordCamp Central site does, a range of days in case of multi-days WordCamps.

Attachments (10)

47798.patch (1.4 KB) - added by imath 5 years ago.
current-screenshot.png (180.8 KB) - added by iandunn 5 years ago.
47798.2.patch (2.0 KB) - added by imath 5 years ago.
47798.3.patch (2.0 KB) - added by Hareesh Pillai 5 years ago.
47798.3.2.patch (2.0 KB) - added by Hareesh Pillai 5 years ago.
Consistent formatting
47798.4.patch (2.0 KB) - added by imath 5 years ago.
47798.4.2.patch (2.0 KB) - added by imath 5 years ago.
Using – at both places
47798.diff (2.0 KB) - added by desrosj 5 years ago.
Capture d’écran 2020-01-21 à 21.13.06.png (68.4 KB) - added by audrasjb 5 years ago.
47798.diff works fine on my side with the API changes
47798.2.diff (2.8 KB) - added by SergeyBiryukov 5 years ago.

Download all attachments as: .zip

Change History (42)

@imath
5 years ago

This ticket was mentioned in Slack in #meta-wordcamp by imath. View the logs.


5 years ago

#2 @casiepa
5 years ago

@imath, I think this will fail if the WordCamp is 31-Aug until 1-Sep (so different month). An extra check is probably needed if the month is different.

#3 @imath
5 years ago

@casiepa thanks a lot for your feedback and « bien vu 😉 », I’ll update the patch so that it does:

  • September 13-14
  • September 30 - October 1

#4 @iandunn
5 years ago

  • Keywords needs-design-feedback added

Here's what the current formatting looks like:

https://core.trac.wordpress.org/attachment/ticket/47798/current-screenshot.png

...and here's what it looks like w/ the patch:

https://cldup.com/H5h3x9a267.png

For meetup events, I think it's important to keep the day of the week, since that's much more important in that context.

It's not as important for WordCamps, though, since they're much further in the future and people have more time to plan ahead.

It looks like the patch already takes that into consideration, but I just wanted to state it explicitly, since that's something that was done intentionally when the widget was built.

Using different formats for different events would be visually inconsistent, though, and that might look odd. I'm not sure what a good solution to is, though. Maybe it's not that big of a deal?

@imath
5 years ago

#5 @imath
5 years ago

To take in account @casiepa 's feedback (the case when a WordCamp is on 2 months) I've just added 47798.2.patch Here's how it looks:

https://cldup.com/-bpWreDQx6.jpg

@Hareesh Pillai
5 years ago

Consistent formatting

#6 @Hareesh Pillai
5 years ago

  • Focuses ui added
  • Keywords needs-testing added

47798.3.patch has two improvements over the previous one -

  1. Added a space between the dates.
  2. 'August' is changed to 'Aug' - to be consistent with the existing format.

#7 @_DorsVenabili
5 years ago

+1 to this request :)

#8 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.3

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


5 years ago

#10 @melchoyce
5 years ago

Super minor, can we be sure to use an en dash instead of a hyphen between date values? https://practicaltypography.com/hyphens-and-dashes.html

#11 @melchoyce
5 years ago

One other comment — the “18 h 00 min” in @imath's last screenshot is confusing, IMO. Is that 18 hours until the event? Or the event itself is 18 hours long? Why is that information important?

#12 @imath
5 years ago

Thanks a lot for your feedback @melchoyce : I haven't touch to how Meetups are displayed, the "18 h 30 min" is the meeting hour in French way of displaying a meeting time.

It depends on the date/hour settings of WordPress. I guess it must be something like "6:30 pm" for en_US. For instance on this new screenshot below, I've used an en dash and changed my WordPress settings to use another way to display hours.

https://cldup.com/xAIr06FPgc.png

47798.4.patch is using an en dash instead of an hyphen when WordCamps are more than one day long.

I've read @hareesh-pillai feedback about using a "short month" format for WordCamps. I think it's important to be consistent with how it is displayed on central.wordcamp.org. That's why I'm using a "full month" format.

Last edited 5 years ago by imath (previous) (diff)

@imath
5 years ago

@imath
5 years ago

Using – at both places

#13 @melchoyce
5 years ago

Ahh okay, thanks for the info! That makes sense.

#14 @mapk
5 years ago

Taking a look at the screenshots from @imath, the latest one looks great! The time makes sense, and I'm all for keeping the full month name as well. Using an en-dash between the days is also correct usage. This looks good!

#15 @karmatosed
5 years ago

  • Keywords needs-design-feedback removed

As this has design feedback, for now removing the keyword.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

@desrosj
5 years ago

#17 @desrosj
5 years ago

This is looking good!

I don't think that the end_date being included in the API response is necessarily a blocker for this to be committed, but ideally the changes for that would land ASAP so proper testing can happen during the beta and RC periods.

I asked for an update over on the meta ticket just to make sure those changes are still realistic for 5.3.

The only change 47798.diff contains is a PHPCS fix.

#18 @francina
5 years ago

  • Keywords needs-testing removed

I removed the needs-testing keyword because the patch can not be tested until the Meta ticket is solved.

#19 @davidbaumwald
5 years ago

  • Keywords commit added

#20 @davidbaumwald
5 years ago

  • Keywords early added
  • Milestone changed from 5.3 to 5.4

With 5.3 Beta 1 in just a few hours and given the meta API change required for this feature, this is being moved for early consideration in 5.4.

#21 @davidbaumwald
5 years ago

  • Keywords commit removed

#22 @dd32
5 years ago

Just noting that the API has been updated to include the end_date field.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

#24 @davidbaumwald
5 years ago

  • Keywords needs-testing added
  • Owner set to desrosj
  • Status changed from new to assigned

@audrasjb
5 years ago

47798.diff works fine on my side with the API changes

#25 @audrasjb
5 years ago

The last patch works fine on my side.

I'm only wondering about the translators context comments. Not a big deal but I believe we usually use something like:

$response_body['events'][ $key ]['formatted_date'] = sprintf(
	/* Translators: %1$s: starting month. %2$d: starting day. %2$d: ending day. %4$d: year */
	__( '%1$s %2$d–%3$d, %4$d' ),
	$month_start,
	date_i18n( __( 'j' ), $timestamp ),
	date_i18n( __( 'j' ), $end_timestamp ),
	date_i18n( __( 'Y' ), $timestamp )
);

Instead of:

/* Translators: %1$s is for the month, %2$d is for the starting day, %2$d is for the ending day, %4$d is for the year */
$response_body['events'][ $key ]['formatted_date'] = sprintf(
	__( '%1$s %2$d–%3$d, %4$d' ),
	$month_start,
	date_i18n( __( 'j' ), $timestamp ),
	date_i18n( __( 'j' ), $end_timestamp ),
	date_i18n( __( 'Y' ), $timestamp )
);

#26 @audrasjb
5 years ago

thanks @SergeyBiryukov, 47798.2.diff looks perfect to me.

#27 @SergeyBiryukov
5 years ago

In: 47798.2.diff:

  • Adjust translator comments for consistency with the rest of core.
  • Add translator comments and context for F, j, Y strings, for consistency with post_submit_meta_box() and a few other places. I'm not 100% sure these should be translatable, but I guess it wouldn't hurt.
  • Call wp_maybe_decline_date() on the resulting date to make sure it's properly declined in locales that require it. Normally, this would happen automatically in date_i18n(), but here we're constructing the date string from several pieces, so need to call the function separately.

@audrasjb: Thanks for the review :)

#28 @SergeyBiryukov
5 years ago

In 47098:

Date/Time: In wp_maybe_decline_date(), add support for a range of days, e.g. February 21–23.

A potential use case is displaying multi-day events in the WordPress Events and News dashboard widget.

See #47798, #48934.

#29 @SergeyBiryukov
5 years ago

  • Keywords commit added; needs-testing removed

47798.2.diff should be good to go.

#30 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 47124:

Dashboard: Improve the appearance of "WordPress Events and News" dashboard widget for multi-day events.

If an upcoming event spans over several days, this information is now properly reflected in the widget.

Props imath, casiepa, iandunn, hareesh-pillai, melchoyce, mapk, desrosj, audrasjb, SergeyBiryukov.
Fixes #47798.

#31 @iandunn
4 years ago

I'm working on porting format_event_data_time() to JavaScript for #51130. 47798.4.2.patch added the $start_month and start_end variables to determine if the date crosses a month boundary.

Do those strings need to be internationalized? They're only used for comparison, not displayed. It doesn't seem like they do, but I want to double check before removing it.

@imath, @SergeyBiryukov

#32 @audrasjb
4 years ago

As soon as they are not displayed and not passed to any hook, I think it's clear those strings don't need to be internationalized.

Note: See TracTickets for help on using tickets.