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

FeaturePanel: Enhancements to the opening hours #516

Merged
merged 6 commits into from
Sep 8, 2024
Merged

FeaturePanel: Enhancements to the opening hours #516

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5ec0520
Enhancements to the opening hours
Dlurak Sep 5, 2024
6bd8eb9
Convert const-enums to train-case
Dlurak Sep 7, 2024
ae5b84b
Merge branch 'master' into opening-hours-enhancements
Dlurak Sep 7, 2024
7fc33a6
Change the condition for splitting
Dlurak Sep 8, 2024
b4c70de
Remove the midnight translation
Dlurak Sep 8, 2024
def13c2
Merge branch 'master' into opening-hours-enhancements
Dlurak Sep 8, 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
28 changes: 18 additions & 10 deletions src/components/FeaturePanel/renderers/OpeningHoursRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { ToggleButton } from '../helpers/ToggleButton';
import { parseOpeningHours } from './openingHours';
import { SimpleOpeningHoursTable } from './openingHours/types';
import { useFeatureContext } from '../../utils/FeatureContext';
import { Status } from './openingHours/complex';

const Table = styled.table`
margin: 1em;
Expand All @@ -25,20 +26,27 @@ const weekDays = t('opening_hours.days_su_mo_tu_we_th_fr_sa').split('|');
const formatTimes = (times: string[]) =>
times.length ? times.map((x) => x.replace(/:00/g, '')).join(', ') : '-';

const formatDescription = (isOpen: boolean, days: SimpleOpeningHoursTable) => {
const formatDescription = (status: Status, days: SimpleOpeningHoursTable) => {
const timesByDay = Object.values(days);
const day = new Date().getDay();
const today = timesByDay[day];
const todayTime = formatTimes(today);
const isOpenedToday = today.length;

if (isOpen) {
return t('opening_hours.open', { todayTime });
switch (status) {
case 'opened':
return t('opening_hours.open', { todayTime });
case 'closed':
return isOpenedToday
? t('opening_hours.now_closed_but_today', { todayTime })
: t('opening_hours.today_closed');
case 'opens soon':
return isOpenedToday
? t('opening_hours.opens_soon_today', { todayTime })
: t('opening_hours.opens_soon');
case 'closes soon':
return t('opening_hours.closes_soon');
}

const isOpenedToday = today.length;
return isOpenedToday
? t('opening_hours.now_closed_but_today', { todayTime })
: t('opening_hours.today_closed');
};

const OpeningHoursRenderer = ({ v }) => {
Expand All @@ -51,7 +59,7 @@ const OpeningHoursRenderer = ({ v }) => {
state: '',
});
if (!openingHours) return null;
const { daysTable, isOpen } = openingHours;
const { daysTable, status } = openingHours;

const { ph, ...days } = daysTable;
const timesByDay = Object.values(days).map((times, idx) => ({
Expand All @@ -69,7 +77,7 @@ const OpeningHoursRenderer = ({ v }) => {
<>
<AccessTime fontSize="small" />
<div suppressHydrationWarning>
{formatDescription(isOpen, daysTable)}
{formatDescription(status, daysTable)}
<ToggleButton onClick={toggle} isShown={isExpanded} />
{isExpanded && (
<Table>
Expand Down
83 changes: 83 additions & 0 deletions src/components/FeaturePanel/renderers/openingHours/__tests__/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { splitDateRangeAtMidnight } from '../utils';

describe('splitDateRangeAtMidnight', () => {
it('should not split it when it isn not needed', () => {
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 4, 4),
new Date(2024, 4, 4, 6, 6),
]),
).toEqual([[new Date(2024, 4, 4, 4, 4), new Date(2024, 4, 4, 6, 6)]]);
});

it('should not split it at midnight into x sub ranges', () => {
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 4, 4),
new Date(2024, 4, 5, 6, 6),
]),
).toEqual([
[new Date(2024, 4, 4, 4, 4), new Date(2024, 4, 5, 0, 0)],
[new Date(2024, 4, 5, 0, 0), new Date(2024, 4, 5, 6, 6)],
]);

expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 4, 4),
new Date(2024, 4, 6, 6, 6),
]),
).toEqual([
[new Date(2024, 4, 4, 4, 4), new Date(2024, 4, 5, 0, 0)],
[new Date(2024, 4, 5, 0, 0), new Date(2024, 4, 6, 0, 0)],
[new Date(2024, 4, 6, 0, 0), new Date(2024, 4, 6, 6, 6)],
]);
});

it('should work when the dates start/end at midnight', () => {
// Test case where the start and end time is exactly at midnight
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 0, 0),
new Date(2024, 4, 5, 0, 0),
]),
).toEqual([[new Date(2024, 4, 4, 0, 0), new Date(2024, 4, 5, 0, 0)]]);

// Test case where start time is exactly at midnight and end time is later the same day
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 0, 0),
new Date(2024, 4, 4, 23, 59),
]),
).toEqual([[new Date(2024, 4, 4, 0, 0), new Date(2024, 4, 4, 23, 59)]]);

// Test case where start time is just before midnight and end time is exactly at midnight
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 23, 59),
new Date(2024, 4, 5, 0, 0),
]),
).toEqual([[new Date(2024, 4, 4, 23, 59), new Date(2024, 4, 5, 0, 0)]]);

// Test case where range starts and ends on different midnights
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 0, 0),
new Date(2024, 4, 6, 0, 0),
]),
).toEqual([
[new Date(2024, 4, 4, 0, 0), new Date(2024, 4, 5, 0, 0)],
[new Date(2024, 4, 5, 0, 0), new Date(2024, 4, 6, 0, 0)],
]);

// Test case where range starts at midnight and ends at a time that is not midnight
expect(
splitDateRangeAtMidnight([
new Date(2024, 4, 4, 0, 0),
new Date(2024, 4, 5, 12, 0),
]),
).toEqual([
[new Date(2024, 4, 4, 0, 0), new Date(2024, 4, 5, 0, 0)],
[new Date(2024, 4, 5, 0, 0), new Date(2024, 4, 5, 12, 0)],
]);
});
});
40 changes: 36 additions & 4 deletions src/components/FeaturePanel/renderers/openingHours/complex.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import OpeningHours from 'opening_hours';
import { isInRange } from './utils';
import { splitDateRangeAtMidnight } from './utils';
import { Address, SimpleOpeningHoursTable } from './types';
import { LonLat } from '../../../../services/types';
import { intl } from '../../../../services/intl';

type Weekday = keyof SimpleOpeningHoursTable;
const WEEKDAYS: Weekday[] = ['su', 'mo', 'tu', 'we', 'th', 'fr', 'sa', 'ph'];
Expand All @@ -16,7 +17,25 @@ const weekdayMappings: Record<string, Weekday> = {
};

const fmtDate = (d: Date) =>
d.toLocaleTimeString(undefined, { hour: 'numeric', minute: 'numeric' });
d.toLocaleTimeString(intl.lang, { hour: 'numeric', minute: 'numeric' });

export type Status = 'opens soon' | 'closes soon' | 'opened' | 'closed';

const getStatus = (opensInMins: number, closesInMins: number): Status => {
const isOpened = opensInMins <= 0 && closesInMins >= 0;

if (!isOpened && opensInMins <= 15) {
return 'opens soon';
}
if (isOpened && closesInMins <= 15) {
return 'closes soon';
}
if (isOpened) {
return 'opened';
}

return 'closed';
};

export const parseComplexOpeningHours = (
value: string,
Expand All @@ -36,8 +55,12 @@ export const parseComplexOpeningHours = (
oneWeekLater.setDate(oneWeekLater.getDate() + 7);

const intervals = oh.getOpenIntervals(today, oneWeekLater);
const splittedIntervals = intervals.flatMap(([openingDate, endDate]) =>
splitDateRangeAtMidnight([openingDate, endDate]),
Copy link
Owner

@zbycz zbycz Sep 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ad splitting interval at midnight) Are you really sure, it is better? It feels natural for me to read an opening hour like Monday 17-02, I think it is clear that is over the midnight. Why did you really choose to split it into two?

I would guess the reason could be to compute the "closes soon" more properly? If this is the case, I would propose to still show the "human version" and just compute the opening by the split time.
Maybe you have some other reason, if so, please tell 🙂

I added some screenshots to your opening post. Especially in English it is in my opinion unreadable. (6PM - 2PM would be fine.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting view on it, which I agree with for your examples!
I primarily thought about cases where one interval starts at one day and ends at least two days later (e.g. 24/7). Previously it would show only one day as opened and all the other ones as closed - now each day gets exactly what's correct for the day.

Old New
Old New

Copy link
Owner

Choose a reason for hiding this comment

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

I see. Thats why you decided to split it by midnight. Now it makes sense 🙂 Please if possible, (next time) try to add more examples to the PR description, there is so much variation in OSM data.

So – I agree, that your fix to the 24/7 hours is defintely useful, though I still think you should not change the 18-02 to the split version. Do you agree? Can you find a way how to code it? If you like, we may discuss it over discord/videocall 🙂

Copy link
Owner

Choose a reason for hiding this comment

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

Also one another idea – the 00-00 is rather unclear to me. It was like this before, so feel free to leave it alone, or leave it to a followup. Though, when we are about to show 00-00, I would strongly prefer 00-24 – what do you think? 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Formatting Enhancements

  • Added German and English translations for "midnight" (i.e., 24/12 PM).
  • Added German and English translations for "all day" (i.e., 24 hours).

Splitting Logic Update

  • The system now only splits intervals if they extend beyond 5 AM on the following day. This means that:
    • Intervals like 24/7 will be split.
    • Intervals like 18-02 will remain unchanged.

I chose 5 AM as the split point to balance clarity and usability. It ensures that intervals that extend slightly into the next day remain unaltered while making it clear when an interval overlaps into the morning or midday.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here are some examples:

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks. Translation of 00-00 to 24 hours is a good solution.

Though, please do not translate midnight in other intervals. When it is 18-00, it is clear already, and adding this to translations will brake all the languages that are not translated yet (becaues english would be used as default). What is even worse is, that the user can change locale to the other one, and it is not really 1:1 to the language then 😅

After you fix this, feel free to merge it.

btw, I prefixed the PR title with "FeaturePanel:" – please try to add the folder / bigger component where is your PR located. It helps me creating changelogs.

);

const grouped = WEEKDAYS.map((w) => {
const daysIntervals = intervals.filter(
const daysIntervals = splittedIntervals.filter(
([from]) =>
w === weekdayMappings[from.toLocaleString('en', { weekday: 'short' })],
);
Expand All @@ -55,8 +78,17 @@ export const parseComplexOpeningHours = (
}),
) as unknown as SimpleOpeningHoursTable;

const currently = new Date();
const getMinsDiff = (date: Date) =>
Math.round((date.getTime() - currently.getTime()) / 60000);
// intervals are sorted from the present to the future
// so the first one is either currently opened or the next opened slot
const relevantInterval = intervals.find(([, endDate]) => endDate > currently);
const opensInMins = relevantInterval ? getMinsDiff(relevantInterval[0]) : 0;
const closesInMins = relevantInterval ? getMinsDiff(relevantInterval[1]) : 0;

return {
daysTable,
isOpen: intervals.some(([from, due]) => isInRange([from, due], new Date())),
status: getStatus(opensInMins, closesInMins),
};
};
30 changes: 28 additions & 2 deletions src/components/FeaturePanel/renderers/openingHours/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,28 @@
export const isInRange = ([startDate, endDate]: [Date, Date], date: Date) =>
date.getTime() >= startDate.getTime() && date.getTime() <= endDate.getTime();
type DateRange = [Date, Date];

export function splitDateRangeAtMidnight([
startDate,
endDate,
]: DateRange): DateRange[] {
const isSameDay = (date1: Date, date2: Date) =>
date1.getFullYear() === date2.getFullYear() &&
date1.getMonth() === date2.getMonth() &&
date1.getDate() === date2.getDate();

const midnight = new Date(startDate);
midnight.setHours(0, 0, 0, 0);
midnight.setDate(midnight.getDate() + 1);

if (startDate.getTime() === endDate.getTime()) {
return [];
}

if (isSameDay(startDate, endDate)) {
return [[startDate, endDate]];
}

return [
[startDate, midnight],
...splitDateRangeAtMidnight([midnight, endDate]),
];
}
3 changes: 3 additions & 0 deletions src/locales/de.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ export default {
'opening_hours.open': 'Geöffnet: __todayTime__',
'opening_hours.now_closed_but_today': 'Geschlossen, heute: __todayTime__',
'opening_hours.today_closed': 'Heute geschlossen',
'opening_hours.opens_soon': 'Öfnet bald',
'opening_hours.opens_soon_today': 'Öffnet bald: __todayTime__',
'opening_hours.closes_soon': 'Schließt bald',
'opening_hours.days_su_mo_tu_we_th_fr_sa': 'Sonntag|Montag|Dienstag|Mittwoch|Donnerstag|Freitag|Samstag',

'map.github_title': 'GitHub',
Expand Down
3 changes: 3 additions & 0 deletions src/locales/vocabulary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ export default {
'opening_hours.open': 'Open: __todayTime__',
'opening_hours.now_closed_but_today': 'Closed now - Open __todayTime__',
'opening_hours.today_closed': 'Closed today',
'opening_hours.opens_soon': 'Opens soon',
'opening_hours.opens_soon_today': 'Opens soon: __todayTime__',
'opening_hours.closes_soon': 'Closes soon',
'opening_hours.days_su_mo_tu_we_th_fr_sa': 'Sunday|Monday|Tuesday|Wednesday|Thursday|Friday|Saturday',
'opening_hours.editor.closed': 'closed',
'opening_hours.editor.create_advanced': 'You may create more detailed opening hours in <link>YoHours tool</link>.',
Expand Down
Loading