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.

Sign up for GitHub

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

Jump to bottom

Nested single day events always shown only with time #2114

Merged
merged 3 commits into from
Oct 24, 2024
Merged

Nested single day events always shown only with time #2114

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe9a24c
nested events date & time
dzonidoo Oct 17, 2024
912d837
changes after rewview
dzonidoo Oct 23, 2024
25d59c0
changes after review
dzonidoo Oct 24, 2024
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
6 changes: 4 additions & 2 deletions client/components/Events/EventDateTime.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';

import {superdeskApi} from '../../superdeskApi';
import {IEventItem} from '../../interfaces';
import {IEventItem, IPlanningListItemProps} from '../../interfaces';

import {eventUtils, timeUtils} from '../../utils';

Expand All @@ -13,6 +13,7 @@ interface IProps {
item: IEventItem;
ignoreAllDay?: boolean;
displayLocalTimezone?: boolean;
planningItem?: IPlanningListItemProps;
}

export class EventDateTime extends React.PureComponent<IProps> {
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

So if planningItem.date is null, this will continue working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

const isRemoteTimeZone = timeUtils.isEventInDifferentTimeZone(item);
const withYear = multiDay && start.year() !== end.year();
const localStart = timeUtils.getLocalDate(start, item.dates.tz);
Expand Down Expand Up @@ -79,7 +81,7 @@ export class EventDateTime extends React.PureComponent<IProps> {
</span>
)}
<DateTime
withDate={multiDay}
withDate={(showEventStartDate || multiDay)}
Copy link
Member

Choose a reason for hiding this comment

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

Move multiDay check to showEventStartDate function

withYear={withYear}
date={start}
{...commonProps}
Expand Down
5 changes: 3 additions & 2 deletions client/components/Events/EventDateTimeColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import moment from 'moment-timezone';

import {IEventItem} from '../../interfaces';
import {IEventItem, IPlanningListItemProps} from '../../interfaces';
import {superdeskApi} from '../../superdeskApi';

import {eventUtils, timeUtils} from '../../utils';
Expand All @@ -13,6 +13,7 @@ import {EventDateTime} from './EventDateTime';
interface IProps {
item: IEventItem;
multiRow?: boolean;
planningItem?: IPlanningListItemProps;
}

export class EventDateTimeColumn extends React.PureComponent<IProps> {
Expand All @@ -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} />
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

</Row>
</Column>
);
Expand Down
1 change: 1 addition & 0 deletions client/components/Events/EventItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ class EventItemComponent extends React.Component<IProps, IState> {
<EventDateTimeColumn
item={item}
multiRow={listViewType === LIST_VIEW_TYPE.LIST}
planningItem={this.props.planningItem}
/>
{listViewType === LIST_VIEW_TYPE.SCHEDULE ? null : (
<CreatedUpdatedColumn
Expand Down
1 change: 1 addition & 0 deletions client/components/Planning/PlanningItemWithEvents.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export class PlanningItemWithEvents extends React.Component<IProps, IState> {
{...this.props.getEventProps(event)}
multiSelectDisabled
key={event._id}
planningItem={planningProps}
Copy link
Member

Choose a reason for hiding this comment

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

planningProps would be a more consistent naming for a prop

/>
);
})
Expand Down
1 change: 1 addition & 0 deletions client/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,7 @@ export interface IAssignmentItem extends IBaseRestApiResponse {

export interface IBaseListItemProps<T> {
item: T;
planningItem?: IPlanningListItemProps;
lockedItems: ILockedItems;
session: ISession;
privileges: {[key: string]: number};
Expand Down
5 changes: 5 additions & 0 deletions client/utils/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

return moment(eventDate).format('DD/MM/YYYY') != moment(planningDate).format('DD/MM/YYYY');
Copy link
Member

Choose a reason for hiding this comment

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

it'd be cleaner to use isSame method from moment. We don't care about date format here

}

function eventHasPlanning(event: IEventItem): boolean {
return get(event, 'planning_ids', []).length > 0;
}
Expand Down Expand Up @@ -1355,6 +1359,7 @@ const self = {
canConvertToRecurringEvent,
canUpdateEventRepetitions,
isEventSameDay,
showEventStartDate,
isEventRecurring,
getDateStringForEvent,
getEventActions,
Expand Down
Loading