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

calendar() displays wrong relative date #1411

Closed
kadishmal opened this issue Jan 10, 2014 · 15 comments
Closed

calendar() displays wrong relative date #1411

kadishmal opened this issue Jan 10, 2014 · 15 comments

Comments

@kadishmal
Copy link

Using moment 1.7.2 with languages. The unix time 1389172279000 is Wed Jan 08 2014 09:11:19. Today is Fri Jan 10 2014 05:54:54.

When supplied to moment.calendar(1389172279000); the output is:

  1. last Wednesday at 5:11 PM in local GMT +9 time in English.
  2. 지난주 수요일 오후 5시 11분 same GMT +9 but in Korean.

지난주 means last week, not this week, which in the current context is not correct. It should be this week's Wednesday.

Even in English last refers to last week. I doubted this so checked your source code which has the follow code:

moment.lang('en', {
    calendar : {
        lastDay : '[Yesterday at] LT',
        sameDay : '[Today at] LT',
        nextDay : '[Tomorrow at] LT',
        lastWeek : '[last] dddd [at] LT',
        nextWeek : 'dddd [at] LT',
        sameElse : 'L'
    }
});

Korean localization has a similar code:

calendar : {
    sameDay : '오늘 LT',
    nextDay : '내일 LT',
    nextWeek : 'dddd LT',
    lastDay : '어제 LT',
    lastWeek : '지난주 dddd LT',
    sameElse : 'L'
},

The code clearly points that the word last means lastWeek. Thus Wednesday 08, two days ago from today's Friday is definitely not last week.

@ichernev
Copy link
Contributor

I guess the interpretation of this / next / last differs from culture to culture. In USA, saying this Monday means the Monday that is coming, next Monday would mean the one after that, so last Monday means the one that just passed. If in Korean that is different, feel free to propose a patch. Also note that the last moment release is 2.5.0, and we're not maintaining different branches (meaning -- your proposed patch would land in 2.6.0, not 1.x.x).

@inca
Copy link

inca commented May 29, 2014

Stating 'this' or 'last' in this context is incorrect, IMO, because it introduces some ambiguity which is only clarified by using the verb in the right tense (future or past).

I don't know if I'm wrong about how it goes in the USA, but in Russian saying 'прошлый вторник' (last tuesday) will almost always mean tuesday on the last week (unless future tense is used which, in turn, requires additional declination).

Maybe saying 'вторник на этой неделе' (literally, 'this week's tuesday') is the correct form in this case?

@inca
Copy link

inca commented May 29, 2014

Also, it largely depends on the context — whether the user somehow expects that the date is always in the past (e.g. by looking at the event log) or in the future (e.g. by looking at the todo list). In both examples saying 'this tuesday' would mean different tuesdays :)

@Menelion
Copy link

@inca, I've been thinking about it for a long time. Actually, "В прошлый вторник" (last Tuesday) is correct only in several cases. Let's take a month where the First is Monday. So it's correct only if today is any day between Friday the 5th and Thursday the 11th, and you're referring to Tuesday the 2nd. So probably to patch this we need somehow to take the current day and make a switch to decide whether we say just "Во вторник" (on Tuesday) or "В прошлый вторник" (last Tuesday).

@inca
Copy link

inca commented May 30, 2014

I had given this one another thought, now I'm sure that the correct phrase totally depends on the context — whether the past or future (present simple) tense is used in conjunction with the date.

With this in mind I came to a conclusion: when referring to the nearest week day — regardless of whether it is in the past or in the future — it is correct to say [On] dddd (e.g. "Во вторник", "В среду", etc.). I know, this introduces some sort of ambiguity (same string for two different days), but it is the only behaviour that does not break user's expectations.

Some examples

Let's assume that today is Wednesday the 11th.

Future context

Let's say you are watching the TV program guide or whether forecast (or anything else, but you expect all relative dates to point in present or future). Here's how you expect the dates to be displayed:

  • Wed 11th — today
  • Thu 12th — tomorrow
  • Fri 13th — on friday
  • Sat 14th — on saturday
  • Sun 15th — on sunday
  • Mon 16th — on monday (remember, you don't expect the mondays from the past!)
  • Tue 17th — on tuesday
  • Wed 18th — on next wednesday

Past context

Now let's assume you are watching the event log (or some other context that will never contain dates from the future). Here's how you expect the dates in this case:

  • Wed 11th — today
  • Tue 10th — yesterday
  • Mon 9th — on monday (again!)
  • Sun 8th — on sunday (referring to the sunday on the week that has passed)
  • Sat 7th — on saturday
  • Fri 6th — on friday
  • Thu 5th — on thursday
  • Wed 4th — on last wednesday

Few more notes

  • My observations are only true for Russian locale. Please let me know if they are wrong for USA or any other locales.
  • Due to the fact that calendar() will display the same string for two different days (e.g. for Friday the 6th and Friday the 13th) I think that it is worth mentioning in the documentation that calendar() is not recommended in mixed contexts (where both the past and the future events can occur at the same time).
  • The algorithm seems to be pretty straightforward: all you have to do is to count the distance in days between now and observable date, it does not seem to depend on the same week alignment or any other variables (months borders, year borders, etc.).
  • I can contribute myself if the idea is accepted.

@Menelion
Copy link

Sorry others, but...
@inca, Борис, это всё так :). Однако если у вас нет контекста, то в вашем случае (среда 11-е) про понедельник 9-е я скорее ожидаю "позавчера" (ну или "в понедельник"). А вот про понедельник 16-е и вторник 17-е вполне можно сказать уже "в следующий понедельник (вторник)". Я понимаю, что это необязательно, но — можно ведь! То есть, то самое: it wouldn't break my expectations. Ну мои, во всяком случае, не будет)

@inca
Copy link

inca commented May 30, 2014

@Oire Спасибо за ответ, Андрей!

Моим ожиданиям, скорее, не соответствуют фразы [В прошлый(ое)] dddd.

Приведу фрагмент приложения:

screen shot 2014-05-30 at 15 43 38

Здесь датам, которые отмечены на календаре цветом, соответствуют записи в журнале. Даты в журнале, которые выводятся с помощью moment().calendar(), вводят в заблуждение пользователей (сегодня 30 мая, пятница).

P.S. Я знаю, что предлагаемое мной решение — это слишком серьезное ограничение (потенциально сломается у пользователей, которые используют calendar в смешанных контекстах). Может быть, писать тогда «вторник на этой неделе», «вторник на следующей неделе» и т.д., указывая на неделю, а не на сам день? Так будет точнее всего, ИМХО.

P.P.S. Sorry for the Russian, guys :)

@Menelion
Copy link

@inca, well, let's pass back to English to be understood by the others :).
In this case maybe is it worth to add a context parameter? Actually, I will try to patch it myself and we'll see. However, I have a stupid question: should I invoke another moment() to get the current day?

@inca
Copy link

inca commented Jun 2, 2014

@Oire Since the time parameter is not passed into the lang().calendar() to allow complete locale-specific re-implementing (see this), I would throw in the third parameter here and use it instead of invoking moment() manually.

As for the context parameter, I guess, it would introduce some sort of a breaking change in the API unless it is bound to the moment instance and have some suitable default value. But still, this would require elaborating a bit more on every existing supported locale.

@ichernev
Copy link
Contributor

ichernev commented Jun 5, 2014

OK, I skimmed through the discussion, and I have to agree talking about weekdays is a complicated topic, and expecting future or past can play a role. If that is the case for most languages we can add this to moment.

@rxaviers
Copy link

rxaviers commented Jun 5, 2014

Yeap, the biggest source of confusion is what the reference is.

If the reference is _this week, we have a context-free distribution. If the reference is _today, we have past or future distributions.

distance (w,d) date reference: this week (context free) reference: today (future) reference: today (past)
(-2,-12) Sat, May 24 - - last Saturday
(-1,-11) Sun, May 25 last Sunday - last Sunday
(-1,-10) Mon, May 26 last Monday - last Monday
(-1,-9) Tue, May 27 last Tuesday - last Tuesday
(-1,-8) Wed, May 28 last Wednesday - last Wednesday
(-1,-7) Thu, May 29 last Thursday - last Thursday
(-1,-6) Fri, May 30 last Friday - [this] Friday
(-1,-5) Sat, May 31 last Saturday - [this] Saturday
(0,-4) Sun, Jun 1 (first day of week in English) [this] Sunday - [this] Sunday
(0,-3) Mon, Jun 2 [this] Monday - [this] Monday
(0,-2) Tue, Jun 3 [this] Tuesday - [this] Tuesday
(0,-1) Wed, Jun 4 (yesterday) [this] Wednesday - [this] Wednesday
(0,0) Thu, Jun 5 (today) [this] Thursday - -
(0,1) Fri, Jun 6 (tomorrow) [this] Friday [this] Friday -
(0,2) Sat, Jun 7 [this] Saturday [this] Saturday -
(1,3) Sun, Jun 8 next Sunday [this] Sunday -
(1,4) Mon, Jun 9 next Monday [this] Monday -
(1,5) Tue, Jun 10 next Tuesday [this] Tuesday -
(1,6) Wed, Jun 11 next Wednesday [this] Wednesday -
(1,7) Thu, Jun 12 next Thursday [this] Thursday -
(1,8) Fri, Jun 13 next Friday next Friday -
(1,9) Sat, Jun 14 next Saturday next Saturday -
(2,10) Sun, Jun 15 - next Sunday -

@rxaviers
Copy link

rxaviers commented Jun 5, 2014

About my findings on CLDR, it has data for three relative values: {-1, 0, 1}. For example:

"thu": {
  "relative-type--1": "last Thursday",
  "relative-type-0": "this Thursday",
  "relative-type-1": "next Thursday"
},

I have found no explicit documentation on how to use these values (the closest I found was this).

@rxaviers
Copy link

rxaviers commented Jun 5, 2014

My personal findings is that this whole story of this|last|next day of the week is always confusing, saved from a few exceptions when both references give the same output. Examples:

_Reference: today-future (the TODO list example)_

Given a TODO list with "Next Wednesday, go to the gym". What day do you expect to go to the gym? Next week's Wed or After-next-week's Wed? Note today is "Thu" as in the above table.

It's confusing, because we'd name it differently depending of the references. Although, if we had "Next Friday, go to the gym". It's clear we're talking about the next week's Fri. Both references would refer the same way to it.

If the TODO list had lots of entries, and we could navigate through all the days until we see next Wed, it would also be clear.

_Reference: today-past (the events log example)_

If we had an event "Last Fri, machine rebooted". What day do you expect your machine rebooted? Last week's Fri or before-next-week's Fri? It's the analogous case as what I wrote above.

_Reference: this = this week (context-free use case)_

So, is this the salvation? Nope. There's one confusing case in here too: "This Sunday". Does it refer to the coming sunday or this week's sunday? The first day of the week (depending on your region's calendar) sounds confusing at least to me.

_Conclusion_

It highly depends on your use case. Confusion could be avoided by avoiding to use it. For example, see github's comments. Each comment has a date. For dates within the [-7,-1] day range, it's shown as day of the week. For everything else, it's shown as a full date. They avoid using "last ...". So, there's no confusion.

@inca
Copy link

inca commented Jun 6, 2014

@rxaviers Great review, thank you! Frankly, I have already gave up using calendar and actually think it should be deprecated for the reasons above.

@ichernev
Copy link
Contributor

OK, so I'm closing this for now. If anybody wants to implement future/past aware calendar feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants