Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

Directive amCalendar doesn't support any parameter #175

Open
Kjir opened this issue Sep 8, 2015 · 6 comments
Open

Directive amCalendar doesn't support any parameter #175

Kjir opened this issue Sep 8, 2015 · 6 comments
Assignees

Comments

@Kjir
Copy link

Kjir commented Sep 8, 2015

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
Copy link
Author

Kjir commented Sep 8, 2015

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
Copy link
Owner

urish commented Sep 8, 2015

hi @kaililleby ,

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

@urish urish added this to the 1.0.0 milestone Sep 15, 2015
@urish urish self-assigned this Sep 15, 2015
@vabatta
Copy link

vabatta commented Mar 12, 2016

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

@urish
Copy link
Owner

urish commented Mar 12, 2016

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!

@vabatta
Copy link

vabatta commented Mar 14, 2016

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

@idoadiv
Copy link

idoadiv commented May 17, 2016

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;
            }])

@urish urish removed this from the 1.0.0 milestone Oct 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants