Replace jscalendar with jquery-ui datetimepicker #628

Closed
wants to merge 5 commits into
from

Projects

None yet

5 participants

@badfiles
Contributor
badfiles commented Aug 9, 2015

What for does plugins/MantisGraph use jscalendar?

@vboctor vboctor and 1 other commented on an outdated diff Aug 10, 2015
config_defaults_inc.php
@@ -1154,15 +1154,19 @@
$g_complete_date_format = 'Y-m-d H:i T';
/**
- * jscalendar date format string
- * go to http://www.php.net/manual/en/function.date.php
- * for detailed instructions on date formatting
+ * jquery-ui datepicker date format string
+ * @global string $g_calendar_js_date_format
+ */
+$g_calendar_js_date_format = 'yy-mm-dd';
@vboctor
vboctor Aug 10, 2015 Member

The changes in the configuration options here should be reflected in the manual as well. I also wonder whether we should be changing the semantics of existing configuration options or in this case deprecate this one in obsolete.php and add $g_calendar_date_format and $g_calendar_time_format. Let's see more opinions.

@dregad
dregad Aug 10, 2015 Member

deprecate this one in obsolete.php and add $g_calendar_date_format and $g_calendar_time_format

+1 for that

@vboctor vboctor commented on an outdated diff Aug 10, 2015
core/print_api.php
@@ -1984,3 +1984,21 @@ function print_max_filesize( $p_size, $p_divider = 1000, $p_unit = 'kb' ) {
. get_filesize_info( $p_size / $p_divider, lang_get( $p_unit ) );
echo '</span>';
}
+
+/**
+ * Print required js for datetimepicker
+ * @return void
+ * @access public
+ */
+function print_datetimepicker_js() {
@vboctor
vboctor Aug 10, 2015 Member

Does this belong better in html_api.php rather than print_api.php? html is where we add the css/js/etc and print is where we print data and other info.

@vboctor
Member
vboctor commented Aug 10, 2015

Shouldn't this change update library/README.libs? Since it affects what libraries we are dependent on?

@vboctor vboctor commented on an outdated diff Aug 10, 2015
config_defaults_inc.php
@@ -1154,15 +1154,19 @@
$g_complete_date_format = 'Y-m-d H:i T';
/**
- * jscalendar date format string
- * go to http://www.php.net/manual/en/function.date.php
- * for detailed instructions on date formatting
+ * jquery-ui datepicker date format string
+ * @global string $g_calendar_js_date_format
+ */
+$g_calendar_js_date_format = 'yy-mm-dd';
+
+/**
+ * jquery-ui datepicker time addon time format string
* @global string $g_calendar_js_date_format
@vboctor
vboctor Aug 10, 2015 Member

This variable name doesn't match the real variable. Also if you rename the other one, then you need to update the similar phpdoc tag as well.

@vboctor
Member
vboctor commented Aug 10, 2015

@badfiles it would be great if you can provide screenshots showing before/after. Also what sort of bugs were there before that are being fixed by this.

How do the old/new controls compare as far as localization? (i.e. # of supported languages).

Please also reference a bug # from the bug tracker that has information about the issue.

@syncguru It would be great if you can have a look at this PR.

@dregad dregad commented on an outdated diff Aug 10, 2015
bug_change_status_page.php
@@ -238,7 +235,8 @@
<?php print_documentation_link( 'due_date' ) ?>
</th>
<td>
- <?php echo '<input ' . helper_get_tab_index() . ' type="text" id="due_date" name="due_date" class="datetime" size="20" maxlength="16" value="' . $t_date_to_display . '" />' ?>
+ <?php echo '<input ' . helper_get_tab_index() . ' type="text" id="due_date" name="due_date" class="datetimepicker" size="20" maxlength="16" value="' . $t_date_to_display . '" />' ?>
+ <script type="text/javascript">$( ".datetimepicker" ).datetimepicker({});</script>
@dregad
dregad Aug 10, 2015 Member

Our CSP rules do not allow inline scripts. This should go to common.js or in another, dedicated file.

@dregad dregad commented on an outdated diff Aug 10, 2015
bug_report_page.php
</span>
+ <script type="text/javascript">$( ".datetimepicker" ).datetimepicker({});</script>
@dregad
dregad Aug 10, 2015 Member

Same comment as above re: CSP

@dregad dregad commented on an outdated diff Aug 10, 2015
bug_update_page.php
@@ -349,7 +346,8 @@
if( !date_is_null( $t_bug->due_date ) ) {
$t_date_to_display = date( config_get( 'calendar_date_format' ), $t_bug->due_date );
}
- echo '<input ' . helper_get_tab_index() . ' type="text" id="due_date" name="due_date" class="datetime" size="20" maxlength="16" value="' . $t_date_to_display . '" />';
+ echo '<input ' . helper_get_tab_index() . ' type="text" id="due_date" name="due_date" class="datetimepicker" size="20" maxlength="16" value="' . $t_date_to_display . '" />';
+ echo '<script type="text/javascript">$( ".datetimepicker" ).datetimepicker({});</script>';
@dregad
dregad Aug 10, 2015 Member

Same comment as above re: CSP

@dregad dregad commented on an outdated diff Aug 10, 2015
manage_proj_ver_edit_page.php
<span class="label-style"></span>
+ <script type="text/javascript">$( ".datetimepicker" ).datetimepicker({});</script>
@dregad
dregad Aug 10, 2015 Member

Same comment as above re: CSP

@dregad
Member
dregad commented Aug 10, 2015

Shouldn't this change update library/README.libs

jscalendar is not in library, the code is just some files in javascript/jscalendar, which should be removed.

I am not sure about how the localization is handled in this PR - jquery-ui-timepicker-english.js and jquery-ui-timepicker-russian.js seem to be custom-built; I believe we should use the i18n files provided with the widget.

Anyway, @badfiles many thanks for this - this has been on my todo list since forever and I could never find the time to implement it...

What for does plugins/MantisGraph use jscalendar?

It's used in MantisGraph/bug_graph_page.php (accessible from view_all_bug_page.php)

@badfiles
Contributor

we should use the i18n files provided with the widget.

For this should not lang_api give RFC language code?

I would call it just g_calendar_date_format

This variable already used for calendar feedback.

It's used in MantisGraph/bug_graph_page.php

But there are no objects on that page having a described in jscalendar.js class.

@atrol
Member
atrol commented Aug 10, 2015

We could use pure HTML5 in future versions, unfortunately a bit too early
http://caniuse.com/#feat=input-datetime

@atrol
Member
atrol commented Aug 10, 2015

For this should not lang_api give RFC language code?

We could use $g_language_auto_map to implement it.

@dregad
Member
dregad commented Aug 10, 2015

we should use the i18n files provided with the widget.

For this should not lang_api give RFC language code?

Ideally yes, but currently we only store the language and unfortunately not the locale. Maybe we can do an approximation of the proper code to use by performing a reverse lookup on the $g_language_auto_map array (using a new API function).

Eventually I think we should refactor lang_api to use RFC codes but that's another topic.

Regardless of the approach, not using existing i18n files and buidling our own would be a mistake IMO.

I would call it just g_calendar_date_format

This variable already used for calendar feedback.

Sorry I did not pay attention that we had both a PHP format and a Javascript format. In that case it's maybe best to keep the variable name as it originally was (i.e. $g_calendar_js_date_format, maybe change the comment to reflect the fact that it's a JavaScript format, not necessarily specific to the datepicker) - with apologies for making you go in circles.

It's used in MantisGraph/bug_graph_page.php

But there are no objects on that page having a described in jscalendar.js class.

IIRC the jscalendar buttons are dynamically created by jQuery code (see common.js) based on the text fields having datetime class. You can see it here if you pick Arbitrary dates from the selection list.

Incidentally, this is probably how you should handle things instead of using inline <script> tags

@vboctor
Member
vboctor commented Aug 15, 2015

jscalendar is not in library, the code is just some files in javascript/jscalendar, which should be removed.

Shouldn't we still refer to the website and version that we got these files from? It is a library that we are using but for technical reasons we have to place the JS and CSS files in another location.

@atrol atrol commented on the diff Aug 15, 2015
config_defaults_inc.php
* @global string $g_calendar_js_date_format
*/
-$g_calendar_js_date_format = '\%Y-\%m-\%d \%H:\%M';
+$g_calendar_jq_time_format = 'HH:mm';
@atrol
atrol Aug 15, 2015 Member

Didn't have a deeper look.
Does this mean that we will no longer be able to enter date and time?

@badfiles
badfiles Aug 15, 2015 Contributor

Datepicker and datetimepicker can work together, and format strings combine,
or you can use only datepicker so only g_calendar_js_date_format will be used.
See corresponding $g_calendar_date_format

@badfiles
Contributor

If this replacement fits I will continue to update this PR and remove inline scripts.
i18n is also possible, but I see no use in making it until Mantis supports it.

@vboctor
Member
vboctor commented Jul 20, 2016

@badfiles following up on this, can we get this PR updated, provide screenshots, and other feedback that is still pending.

For the graph, you can see the page that requires date time picker by installing the plugin, then going to View Issues page and click on "Issue Trends" just about the issue list.

@badfiles
Contributor

Screenshot of the new date-time picker.
screenshot from 2016-07-25 16-03-37

@dregad
Member
dregad commented Aug 16, 2016

@badfiles can you please rebase this ?

@syncguru
Contributor

@badfiles, @atrol, @vboctor, @dregad: What is the status of this PR?
I prefer using this library: https://github.com/uxsolutions/bootstrap-datepicker over jquery-ui.

@vboctor
Member
vboctor commented Oct 23, 2016

@syncguru I've tried the bootstrap-datepicker and it looks good.

@atrol
Member
atrol commented Oct 23, 2016

@syncguru I had just a short look it. Seems this is just a date picker, but we need a date-time picker.

@badfiles
Contributor

Close this PR in favor to #943

@badfiles badfiles closed this Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment