Baremetrics / calendar

Date range picker for Baremetrics
MIT License
679 stars 78 forks source link

Request: Day of the week starts on... #95

Closed jackmcdade closed 5 years ago

jackmcdade commented 5 years ago

I can see that this has been brought up multiple times (#53 and #58) but it looks like it was always closed down. Is there any way to get this added? Can I help? This is a pretty important UX aspect for international use.

kalepail commented 5 years ago

As I understand it all you need to do is import moment with the locales needed and then select the locale you'd like to load the calendar in.

<script src="js/vendor/jquery.js"></script>
  <!-- <script src="js/vendor/moment.js"></script> -->
  <script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.24.0/moment-with-locales.min.js"></script>

<script>
  moment.locale('en-au');
</script>
jackmcdade commented 5 years ago

Oh interesting. I'll give that a try and report back. 👍

jackmcdade commented 5 years ago

Negative. It formats the date string, but doesn't affect the calendar table at all. This is French, which starts the week on Monday.

image

kalepail commented 5 years ago

What locale are you using?

jackmcdade commented 5 years ago

This is French, which starts the week on Monday.

kalepail commented 5 years ago

According to moment the French calendar starts on Dimanche which Google tells me is Sunday.

...
hooks.defineLocale('fr-ca', {
        months : 'janvier_février_mars_avril_mai_juin_juillet_août_septembre_octobre_novembre_décembre'.split('_'),
        monthsShort : 'janv._févr._mars_avr._mai_juin_juil._août_sept._oct._nov._déc.'.split('_'),
        monthsParseExact : true,
        weekdays : 'dimanche_lundi_mardi_mercredi_jeudi_vendredi_samedi'.split('_'),
        weekdaysShort : 'dim._lun._mar._mer._jeu._ven._sam.'.split('_'),
        weekdaysMin : 'di_lu_ma_me_je_ve_sa'.split('_'),
        weekdaysParseExact : true,
...
kalepail commented 5 years ago

Hmm looks like all the locales follow the convention of starting the weekday params on Sunday. The offset must be somewhere else. Looking.

kalepail commented 5 years ago

If you switch to en-gb or en-au does it start on a Monday? That's what I tested.

kalepail commented 5 years ago

Are you using fr-ca or fr-ch The fr-ch has the offset while fr-ca does not.

week : {
            dow : 1, // Monday is the first day of the week.
            doy : 4  // The week that contains Jan 4th is the first week of the year.
        }
jackmcdade commented 5 years ago

I tried both and they both have the same week start. Dunno man.

kalepail commented 5 years ago

Works perfectly for me.

screen shot 2019-02-06 at 1 55 25 pm

All I can think is either you haven't included the locale version of moment <script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.24.0/moment-with-locales.min.js"></script> or else you haven't set moment.locale('fr-ch'); soon enough before the calendar tries to generate and render.

kalepail commented 5 years ago

But then you wouldn't get the language translation either right? /shrug I assume you are on v.1.0.14?

jackmcdade commented 5 years ago

Yep, I'm on 1.0.14. I can see the locale is loaded, it handles the translation, but no change on the calendar. Here's fr-ch, set in our bootstrapping mounted() method.

image

jackmcdade commented 5 years ago

It's worth mentioning that moment.localeData().firstDayOfWeek() shows 1, as expected, so it seems there's a disconnect between the moment instance and Calendar.

We're importing moment with webpack and binding it to window, and then setting .locale(). Maybe Calender is newing up an instance of moment or something?

jackmcdade commented 5 years ago

Figured it out. We were binding moment to window, but Calendar wasn't using that instance. Binding to Vue.moment and leveraging that instance globally did the trick.

Thanks for the help and moral support @tyvdh, much appreciated!