urish/angular-moment

Directive amCalendar doesn't support any parameter

Opened this issue · 6 comments

Kjir commented

The directive for amCalendar doesn't support any parameter, while the moment.js calendar() method offers the possibility to customize the format string since version 2.10.5. angular-moment should allow to pass some parameters to customize the date format.

cfr. http://momentjs.com/docs/#/displaying/calendar-time/

Kjir commented

A side issue is that to be consistent, the format should be the second argument of amCalendar, which would also make the most common usage simpler. This would mean a breaking change of interface.

The other option would be to append a new parameter to the end of the filter, but this would mean that in order to change just the format the user would need to pass 2 undefined parameters every time, which is kinda ugly. Also it would be inconsistend with other directives.

There are a couple of additional options:

  1. Check if the second parameter is a string or an object, and if it's a string it means that we should consider it to be a preprocessor, but we log a deprecation warning. This seems not so elegant...
  2. Add a configuration option to angularMomentConfig, avoiding the need for an additional parameter. This should be done anyway to provide a default, nonetheless we should also have the parameter.

Each solution has its pros and cons. IMHO keeping the consistency is more important than not breaking the interface, but that would mean to bump a major version for a somewhat minor feature.

urish commented

hi @kaililleby ,

thank you for your inputs! Please see the discussion in #174, about removing the preprocessors and simplifying the API...

There is somewhat a date for this feature to be ready? I would like to use it!

urish commented

Feel free to send a pull request :-)

If you do, please make sure to add a unit-test and update README with the new parameters.

Thanks!

I just solved changing the logic for the date in order to use just | amCalendar without params :)

Quick fix

.filter('amCalendar', ['moment', 'amMoment', 'angularMomentConfig', function (moment, amMoment, angularMomentConfig) {
                function amCalendarFilter(value, referenceTime, formats) {
                    if (isUndefinedOrNull(value)) {
                        return '';
                    }

                    var date = amMoment.preprocessDate(value);
                    return date.isValid() ? date.calendar(referenceTime, formats) : '';
                }

                // Since AngularJS 1.3, filters have to explicitly define being stateful
                // (this is no longer the default).
                amCalendarFilter.$stateful = angularMomentConfig.statefulFilters;

                return amCalendarFilter;
            }])