Nested single day events always shown only with time #2114
Conversation
| @@ -23,6 +24,7 @@ export class EventDateTime extends React.PureComponent<IProps> { | |||
| const end = eventUtils.getEndDate(item); | |||
| const isAllDay = eventUtils.isEventAllDay(start, end); | |||
| const multiDay = !eventUtils.isEventSameDay(start, end); | |||
| const showEventStartDate = eventUtils.showEventStartDate(start, this.props.planningItem?.date); | |||
There was a problem hiding this comment.
So if planningItem.date is null, this will continue working?
| @@ -23,7 +24,7 @@ export class EventDateTimeColumn extends React.PureComponent<IProps> { | |||
| return ( | |||
| <Column border={false} className="flex-justify--start sd-padding-t--1"> | |||
| <Row classes="sd-margin--0"> | |||
| <EventDateTime item={this.props.item} /> | |||
| <EventDateTime item={this.props.item} planningItem={this.props.planningItem} /> | |||
There was a problem hiding this comment.
Why is this component called EventDateTime and we're passing an event item and a planningItem? If it's meant to show both planning & event item times you should rename it accordingly
There was a problem hiding this comment.
The component only renders the date and time of the event, but conditionally, when the event's date differs from the planning item's date.
client/utils/events.ts
Outdated
| @@ -80,6 +80,10 @@ function isEventSameDay(startingDate: IDateTime, endingDate: IDateTime): boolean | |||
| return moment(startingDate).format('DD/MM/YYYY') === moment(endingDate).format('DD/MM/YYYY'); | |||
| } | |||
|
|
|||
| function showEventStartDate(eventDate: IDateTime, planningDate: IDateTime): boolean { | |||
There was a problem hiding this comment.
Since up in the code you're passing a possibly undefined planningDate it only makes sense to make this optional here, and handle the case where planningDate is undefined explicitly
|
@tomaskikutis you can take a look as well if you want, but I feel confident in the review and that this is a simple feature |
|
I'll take a look |
| @@ -79,7 +81,7 @@ export class EventDateTime extends React.PureComponent<IProps> { | |||
| </span> | |||
| )} | |||
| <DateTime | |||
| withDate={multiDay} | |||
| withDate={(showEventStartDate || multiDay)} | |||
There was a problem hiding this comment.
Move multiDay check to showEventStartDate function
client/utils/events.ts
Outdated
| if (planningDate == null) { | ||
| return true; | ||
| } | ||
| return moment(eventDate).format('DD/MM/YYYY') != moment(planningDate).format('DD/MM/YYYY'); |
There was a problem hiding this comment.
it'd be cleaner to use isSame method from moment. We don't care about date format here
| @@ -69,6 +69,7 @@ export class PlanningItemWithEvents extends React.Component<IProps, IState> { | |||
| {...this.props.getEventProps(event)} | |||
| multiSelectDisabled | |||
| key={event._id} | |||
| planningItem={planningProps} | |||
There was a problem hiding this comment.
planningProps would be a more consistent naming for a prop
dzonidoo
merged commit 73d2311
into
superdesk:feature/multiple-events-in-planning
STT-93
Front-end checklist
memoorPureComponentto define new React components (and updates existing usages in modified code segments)lodash.getwith optional chaining and nullish coalescing for modified code segmentssuperdeskApi)superdesk-ui-frameworkandsuperdeskApiwhen possible instead of using ones defined in this repository.planningApiwhere it is possible to usesuperdeskApiplanningApi)