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

[WIP] Eonasdan datetimepicker update v4.7.14 #149

Closed
wants to merge 7 commits into from
Closed

[WIP] Eonasdan datetimepicker update v4.7.14 #149

wants to merge 7 commits into from

Conversation

nlzet
Copy link

@nlzet nlzet commented Mar 12, 2015

See #138 for the previous conversation. Here is a table the datepicker option changes

Option Changes
dp_max_date default value changed from null to false
dp_default_date default value changed from '' to false
dp_disabled_dates default value changed from array() to false
dp_enabled_dates default value changed from array() to false
dp_icons default value changed with extra icons for previous, next, today and clear
dp_show_today renamed to dp_show_today_button
dp_language renamed to dp_locale
dp_minute_stepping renamed to dp_stepping
dp_use_minutes removed
dp_use_seconds removed
dp_pick_time removed, this is no longer a boolean option, this is now defined by the date format.

For the dp_pick_time I now used the $this->getName() check to enable/disable the time picker. I think this shouldn't be defined by an option, but by the form type ?

'dp_use_current' => true,
'dp_min_date' => '1/1/1900',
'dp_max_date' => null,
'dp_show_today' => true,
Copy link
Member

Choose a reason for hiding this comment

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

we cannot remove the value, we need to be BC compatible. Just convert the values.

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean I have to keep using the option dp_show_today and convert it to dp_show_today_button ?

What about the removed options dp_use_minutes, dp_use_seconds and dp_pick_time ?

@nlzet
Copy link
Author

nlzet commented Mar 17, 2015

ping @rande, could you give some more details about my question, so I can finish this PR.

@webdevilopers
Copy link
Contributor

With the current dev-master the dp_use_seconds already has no effect when setting it on the field:

$formMapper
    ->add('bundleStatus', 'sonata_type_datetime_picker', array(
        'dp_side_by_side' => true,
       'dp_use_seconds' => false
    ));

The generated html includes the option but set to true:

 jQuery(function ($) {
$('#dtp_s5512b911e3621_bundleStatus').datetimepicker({"pickTime":true,"useCurrent":true,"minDate":"1\/1\/1900","maxDate":null,"showToday":true,"language":"de","defaultDate":"","disabledDates":[],"enabledDates":[],"icons":{"time":"glyphicon glyphicon-time","date":"glyphicon glyphicon-calendar","up":"glyphicon glyphicon-chevron-up","down":"glyphicon glyphicon-chevron-down"},"useStrict":false,"daysOfWeekDisabled":[],"useMinutes":true,"minuteStepping":1,"sideBySide":true,"useSeconds":true});
}); 

https://github.com/sonata-project/SonataCoreBundle/blob/master/Form/Type/BasePickerType.php#L57

I guess this issue is no longer relevant when this PR is merged @nlzet ?
How to disable seconds then?

@nlzet
Copy link
Author

nlzet commented Mar 25, 2015

@webdevilopers Hmm you are right, this option is always automatically set, so specifying it has no use.
Disabling seconds or minutes in the new version is done by the date / time format

But options like dp_use_minutes and dp_pick_time are a different story. I can keep them for to prevent BC errors, but they won't have any use.

I will make an update so the renamed options will be transformed to the new options.

@webdevilopers
Copy link
Contributor

Thanks @nlzet .

When using the time format instead will a data transformer automatically add seconds as :00 to a field in order to save it as a full DATETIME value in database?

@nlzet nlzet changed the title [WIP] Eonasdan datetimepicker update v4.0.0 [WIP] Eonasdan datetimepicker update v4.7.14 Apr 1, 2015
@nlzet
Copy link
Author

nlzet commented Apr 1, 2015

@webdevilopers, the format is now correctly set on the datepicker. I think this library will create BC issues because the datepicker and datetimepicker now actually are the same, the type no longer defines whether to use time or not. Should I continue?

Also there is a version 4.0.0 and 4.7.14 of the plugin, I now updated it to the second one.

Do you have an idea how sonata works with the locale of a date? In my current implementation, when using dp_locale "nl" it fails to save some dates

@webdevilopers
Copy link
Contributor

To be honest I have no idea. I guess @rande can tell you.

@pborreli
Copy link
Contributor

pborreli commented May 6, 2015

what is the status of this PR ? I have issue with actuel one too.

@nlzet
Copy link
Author

nlzet commented May 7, 2015

Well I didn't continue and I don't know if I should. There are new features but there are a lot of BC issues. Maybe this should be targeted for a new version.

What do you think @rande ?

@soullivaneuh
Copy link
Member

There are new features but there are a lot of BC issues. Maybe this should be targeted for a new version.

Indeed. Should be a new major (e.g. 3.0).

@nlzet no possibility to keep BC with a deprecation system?

@nlzet
Copy link
Author

nlzet commented Jun 12, 2015

@soullivaneuh no, I don't think it is possible, I tried but I kept going into new BC problems. I think it is wiser to wait for 3.0.

@soullivaneuh
Copy link
Member

@nlzet well, we'll ping you when a major branch will be started. 👍

Please see: sonata-project/SonataAdminBundle#3053

@SonataCI
Copy link
Collaborator

SonataCI commented Aug 5, 2015

Can one of the admins verify this patch?

@nlzet
Copy link
Author

nlzet commented Sep 15, 2015

@rande, @soullivaneuh I saw there are currently some milestones awaiting for a new 2.4 release for the SonataAdminBundle: https://github.com/sonata-project/SonataAdminBundle/milestones/2.4

I'm unsure but on this issue we said we would wait for version 3.0 on the core bundle, is the 3.0 core version related to the 2.4 release of the admin bundle or will this be in a later stadium ?

@soullivaneuh
Copy link
Member

is the 3.0 core version related to the 2.4 release

No. 3.0 will be shipped when we will have to introduce BC break. Version constraint are not defined in advance AFAIK.

@nlzet
Copy link
Author

nlzet commented Sep 15, 2015

Okay, clear. Just ping me when this day comes.

@OskarStark
Copy link
Member

Fixes #219

@OskarStark OskarStark added this to the 3.0 milestone Feb 10, 2016
@OskarStark
Copy link
Member

@nlzet can you please change the docs and add the BC break stuff to the UPGRADE.md?

Thank you very much

@nlzet
Copy link
Author

nlzet commented Feb 10, 2016

@OskarStark I will look into this, probably next week or after. I see that there are already some new versions and features in the eonasdan-datetimepicker, and also there is a work in progress for a new major release. Probably worth looking a little bit deeper to see what is actually going on and what could be useful for Sonata.

Is there a release date target for this PR / the 3.0 milestone?

@OskarStark
Copy link
Member

glad to hear that @nlzet

not yet, but we would like to tag the next next MINOR, and after that we will tag the next major too

thank you 👍

@soullivaneuh soullivaneuh removed this from the 3.0 milestone Apr 22, 2016
'dp_disabled_dates' => false,
'dp_enabled_dates' => false,
'dp_icons' => array(
'time' => 'glyphicon glyphicon-time',
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like glyphicon was replaced by font-awesome

@soullivaneuh
Copy link
Member

Please rebase and fill the needs tags.

@core23
Copy link
Member

core23 commented Aug 19, 2016

Please rebase and use the new PR template @nlzet

@nlzet
Copy link
Author

nlzet commented Aug 24, 2016

@soullivaneuh @core23 sorry for the delay. I'm going to plan some time for this.

I'm wondering, since this is outdated and has been under discussion because of BC breaks. Which branch should I target for this feature ? (Probably need a new PR ?)

@core23
Copy link
Member

core23 commented Aug 24, 2016

If you can handle this without a BC break, you should target the 3.x branch, if this can't be done the master branch will be your friend.

@nlzet
Copy link
Author

nlzet commented Aug 24, 2016

@core23 at the time it wasn't possible without BC, but I'll have to re-investige the options of the latest version of eonasdan-datepicker, maybe it is possible now.

Thanks anyway

@core23
Copy link
Member

core23 commented Jan 24, 2017

Can you have a look at this @nlzet ?

@core23
Copy link
Member

core23 commented Jun 14, 2017

Please update or close this @nlzet

@nlzet
Copy link
Author

nlzet commented Jun 14, 2017

Sorry @core23,
I closed this, since I cannot find any time to update this.

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

Successfully merging this pull request may close these issues.

9 participants