New DateTime picker (replaces calendar) #11138

Merged
merged 139 commits into from Nov 21, 2016

Conversation

@dgrammatiko
Member

dgrammatiko commented Jul 14, 2016

UX UI improvements

Summary of Changes

  • Bootstrapified
  • Remove old code for IE<8
  • New simplified popup code
  • The internals of the date calculations are unchanged
  • Base on #707 improved support for non Gregorian calendars
  • New time picker code based on select elements (24 or 12h based)
  • options to display or not: week numbers, today button, year month in one row, days from previous next month (as disabled)
  • Everything is controllable through the calendar field definition (documentation will be done later on)

Preview

screen shot 2016-07-15 at 01 54 13

With time picker
screen shot 2016-07-17 at 00 43 45

Basic view
screen shot 2016-07-15 at 02 11 02

Testing Instructions

You need a 3.7 installation
apply this patch
try various forms (contact, banner, article) and check if the calendar works ok.
edit /Users/dimitris/Documents/github_projects/joomla1/administrator/components/com_content/models/forms/article.xml

        <field name="publish_up"
            type="calendar"
            label="COM_CONTENT_FIELD_PUBLISH_UP_LABEL"
            description="COM_CONTENT_FIELD_PUBLISH_UP_DESC"
            format="%Y-%m-%d %H:%M:%S" size="22"
            filter="user_utc"
            todaybutton="true"
            weeknumbers="false"
            timeformat="24"
            showtime="true"
            filltable="true"
            singleheader="false"
            minyear="-100"
            maxyear="+50" />

and alter different options to check if everything still works ok

Install Persian language and test if that calendar works ok!

Try some 3rd PD components and check if they still functioning ok.

Report any problems

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 15, 2016

Member

has this been tested with non gregorian dates, for example jalali? what would the Persian language pack have to add to its pack to force jalali? Is it rtl aware?

Member

infograf768 commented Jul 15, 2016

has this been tested with non gregorian dates, for example jalali? what would the Persian language pack have to add to its pack to force jalali? Is it rtl aware?

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 15, 2016

Contributor

Does this work with the folowing config options from the language xml

  <firstDay>0</firstDay>
    <weekEnd>0,6</weekEnd>
Contributor

brianteeman commented Jul 15, 2016

Does this work with the folowing config options from the language xml

  <firstDay>0</firstDay>
    <weekEnd>0,6</weekEnd>
@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 15, 2016

Member

@brianteeman I hope so:
check layouts/joomla/form/field/calendar.php lines:190-191

      data-firstday="<?php echo JFactory::getLanguage()->getFirstDay(); ?>"
        data-weekend="<?php echo JFactory::getLanguage()->getWeekEnd(); ?>"

@infograf768 Not yet, (rtl is tested tho). As I said #707 has all the changes needed, so it should be fairly easy to do

Member

dgrammatiko commented Jul 15, 2016

@brianteeman I hope so:
check layouts/joomla/form/field/calendar.php lines:190-191

      data-firstday="<?php echo JFactory::getLanguage()->getFirstDay(); ?>"
        data-weekend="<?php echo JFactory::getLanguage()->getWeekEnd(); ?>"

@infograf768 Not yet, (rtl is tested tho). As I said #707 has all the changes needed, so it should be fairly easy to do

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 15, 2016

Member

Preview RTL:
screen shot 2016-07-15 at 12 47 53

Member

dgrammatiko commented Jul 15, 2016

Preview RTL:
screen shot 2016-07-15 at 12 47 53

@JoomliC

This comment has been minimized.

Show comment
Hide comment
@JoomliC

JoomliC Jul 15, 2016

Contributor

@dgt41 Great work!

Sorry i didn't have time before to check your code you linked me, but next week i will dedicate time to test this, and give all possible feedback!

Just a first issue : missing blank space at the bottom when opened, when field is at the bottom of the page, and because of the white "shadow" top of the fixed status footer.

capture d ecran 2016-07-15 a 13 36 38

Contributor

JoomliC commented Jul 15, 2016

@dgt41 Great work!

Sorry i didn't have time before to check your code you linked me, but next week i will dedicate time to test this, and give all possible feedback!

Just a first issue : missing blank space at the bottom when opened, when field is at the bottom of the page, and because of the white "shadow" top of the fixed status footer.

capture d ecran 2016-07-15 a 13 36 38

@JoomliC

This comment has been minimized.

Show comment
Hide comment
@JoomliC

JoomliC Jul 15, 2016

Contributor

@dgt41 uncommented corrects the position (picker at top), but the position is not correct relative to the form field.

There the result when click on "Calendar 4" button :
capture d ecran 2016-07-15 a 13 50 26

Contributor

JoomliC commented Jul 15, 2016

@dgt41 uncommented corrects the position (picker at top), but the position is not correct relative to the form field.

There the result when click on "Calendar 4" button :
capture d ecran 2016-07-15 a 13 50 26

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 15, 2016

Member

@JoomliC so my calculations are off. It seems I hit that often lately 😄

Member

dgrammatiko commented Jul 15, 2016

@JoomliC so my calculations are off. It seems I hit that often lately 😄

@JoomliC

This comment has been minimized.

Show comment
Hide comment
@JoomliC

JoomliC Jul 15, 2016

Contributor

@dgt41 IMO, maybe set its position to top, and bottom only if not enought space at top ? (could be easier to manage too ;-) )
z-index could be higher i think, to keep it over menu and fixed bottom.
Not time enough to check code this week-end, but will look at it with more attention next week ;-)

Contributor

JoomliC commented Jul 15, 2016

@dgt41 IMO, maybe set its position to top, and bottom only if not enought space at top ? (could be easier to manage too ;-) )
z-index could be higher i think, to keep it over menu and fixed bottom.
Not time enough to check code this week-end, but will look at it with more attention next week ;-)

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 16, 2016

Member

non bootstrap preview:
screen shot 2016-07-16 at 16 07 04

Member

dgrammatiko commented Jul 16, 2016

non bootstrap preview:
screen shot 2016-07-16 at 16 07 04

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 16, 2016

Member

@infograf768 Jalali is almost ready:
screen shot 2016-07-16 at 21 05 01

Member

dgrammatiko commented Jul 16, 2016

@infograf768 Jalali is almost ready:
screen shot 2016-07-16 at 21 05 01

+}
+
+// If a known filter is given use it.
+switch (strtoupper($filter))

This comment has been minimized.

@Fedik

Fedik Jul 16, 2016

Contributor

I think would be good to keep this inside the field file, as it part of the field logic,
Also allow override this can lead to unexpected bugs, related to wrong timezone.

@Fedik

Fedik Jul 16, 2016

Contributor

I think would be good to keep this inside the field file, as it part of the field logic,
Also allow override this can lead to unexpected bugs, related to wrong timezone.

+ placeholder="<?php echo empty($description) ? null : $description; ?>"/>
+ <button type="button" class="btn btn-secondary"
+ id="<?php echo $id; ?>_btn"
+ data-inputfield="<?php echo $id; ?>"

This comment has been minimized.

@Fedik

Fedik Jul 16, 2016

Contributor

I have a hope the script does not depend from the input id? :neckbeard:

@Fedik

Fedik Jul 16, 2016

Contributor

I have a hope the script does not depend from the input id? :neckbeard:

This comment has been minimized.

@dgrammatiko

dgrammatiko Jul 16, 2016

Member

The initialisation is done with the class:

var calendars = document.getElementsByClassName("field-calendar");

https://github.com/dgt41/joomla-cms/blob/83da18f968974a1c0020ebe4a63fdbfd630efcba/media/system/js/calendar-setup-vanilla.js#L61

But the code is also using the id but I think I can eliminate that as well 😉

@dgrammatiko

dgrammatiko Jul 16, 2016

Member

The initialisation is done with the class:

var calendars = document.getElementsByClassName("field-calendar");

https://github.com/dgt41/joomla-cms/blob/83da18f968974a1c0020ebe4a63fdbfd630efcba/media/system/js/calendar-setup-vanilla.js#L61

But the code is also using the id but I think I can eliminate that as well 😉

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Jul 16, 2016

Contributor

@dgt41 great job!
I already have some notes :neckbeard:, but first I will try some tests 😉

Contributor

Fedik commented Jul 16, 2016

@dgt41 great job!
I already have some notes :neckbeard:, but first I will try some tests 😉

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 17, 2016

Member

@Fedik I think this is what you meant:

        // Get all the calendar fields
        var calendars = document.getElementsByClassName("field-calendar");

        // Loop to initialize them all
        for (index = 0, len = calendars.length; index < len; ++index) {
            JoomlaCalendar.setup(calendars[index]);
        }

Now sub-forms should be happy 😉

Member

dgrammatiko commented Jul 17, 2016

@Fedik I think this is what you meant:

        // Get all the calendar fields
        var calendars = document.getElementsByClassName("field-calendar");

        // Loop to initialize them all
        for (index = 0, len = calendars.length; index < len; ++index) {
            JoomlaCalendar.setup(calendars[index]);
        }

Now sub-forms should be happy 😉

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Jul 17, 2016

Contributor

@dgt41 yes, this looks better, thanks!

Contributor

Fedik commented Jul 17, 2016

@dgt41 yes, this looks better, thanks!

+ ************************** Initialize *******************************
+ *********************************************************************/
+document.onreadystatechange = function () {
+ if (document.readyState == "interactive") {

This comment has been minimized.

@Fedik

Fedik Jul 17, 2016

Contributor

you do not need to wrap whole thing in to if(), it is hard to support.
can do more easy:

if (document.readyState !== 'interactive') {
 return;
}
...
code here
...
@Fedik

Fedik Jul 17, 2016

Contributor

you do not need to wrap whole thing in to if(), it is hard to support.
can do more easy:

if (document.readyState !== 'interactive') {
 return;
}
...
code here
...
@tomartailored

This comment has been minimized.

Show comment
Hide comment
@tomartailored

tomartailored Jul 19, 2016

I have tested this item successfully on 44589ad

I have Tested successfully this issue


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11138.

I have tested this item successfully on 44589ad

I have Tested successfully this issue


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11138.

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Jul 19, 2016

Contributor

@dgt41 it looks like we have merge conflicts.

Contributor

zero-24 commented Jul 19, 2016

@dgt41 it looks like we have merge conflicts.

@JoomliC

This comment has been minimized.

Show comment
Hide comment
@JoomliC

JoomliC Jul 19, 2016

Contributor

@tomartailored @zero-24 this PR is not yet for "validating" testing, but for first tests, feedback, and improvements ;-)

@dgt41 Just started some tests, and when set by default (no optional attributes) <field name="color" type="calendar" label="Calendar default"/>, you have the time picker, but when a date is selected and time set or not, the input contains only date (not possible to set time).
Maybe due to the default date format %Y-%m-%d to be changed to %Y-%m-%d %H:%M:%S here : https://github.com/joomla/joomla-cms/pull/11138/files#diff-b01d47e3fbe4a8e5fb4987f4df64b291R961

In the same time, a question:
Wouldn't it be easier to have an hidden input to save the date in UTC iso format, and to display the date in visible input depending of the format attribute and language ? (could make it easier to manage with Jalali calendar...) 💡

Contributor

JoomliC commented Jul 19, 2016

@tomartailored @zero-24 this PR is not yet for "validating" testing, but for first tests, feedback, and improvements ;-)

@dgt41 Just started some tests, and when set by default (no optional attributes) <field name="color" type="calendar" label="Calendar default"/>, you have the time picker, but when a date is selected and time set or not, the input contains only date (not possible to set time).
Maybe due to the default date format %Y-%m-%d to be changed to %Y-%m-%d %H:%M:%S here : https://github.com/joomla/joomla-cms/pull/11138/files#diff-b01d47e3fbe4a8e5fb4987f4df64b291R961

In the same time, a question:
Wouldn't it be easier to have an hidden input to save the date in UTC iso format, and to display the date in visible input depending of the format attribute and language ? (could make it easier to manage with Jalali calendar...) 💡

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 19, 2016

Member

@zero-24 these are unit tests that are failing

Member

dgrammatiko commented Jul 19, 2016

@zero-24 these are unit tests that are failing

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 19, 2016

Member

@JoomliC are you spying on my computer? 😄
I mean I had same idea of setting a hidden field. Ok I worked on it a bit more and now this is a data-alt-value="xxxxx"
screen shot 2016-07-19 at 19 10 30

Member

dgrammatiko commented Jul 19, 2016

@JoomliC are you spying on my computer? 😄
I mean I had same idea of setting a hidden field. Ok I worked on it a bit more and now this is a data-alt-value="xxxxx"
screen shot 2016-07-19 at 19 10 30

@JoomliC

This comment has been minimized.

Show comment
Hide comment
@JoomliC

JoomliC Jul 19, 2016

Contributor

@JoomliC are you spying on my computer? 😄

@dgt41 you didn't know i did ? 😆
Good idea about the data-alt-value (i thought of the hidden input, as managing timezones, non-gregorian calendars, etc... is not easy in the input directly, as better to always save datetime in UTC iso format ;-) ).
And doing so would be an improvement of what is done today and will prevent many issues in the future ;-)

Contributor

JoomliC commented Jul 19, 2016

@JoomliC are you spying on my computer? 😄

@dgt41 you didn't know i did ? 😆
Good idea about the data-alt-value (i thought of the hidden input, as managing timezones, non-gregorian calendars, etc... is not easy in the input directly, as better to always save datetime in UTC iso format ;-) ).
And doing so would be an improvement of what is done today and will prevent many issues in the future ;-)

media/system/js/calendar-vanilla.js
+ * @license GNU General Public License version 2 or later; see LICENSE.txt
+ */
+var JoomlaCalendar = function (selector) {
+ 'use strict';

This comment has been minimized.

@Fedik

Fedik Jul 20, 2016

Contributor

better approach is to wrap the code in the file in to the anonymous function, see core.js for example:

!(function(){
  'use strict';
  ... 
  here is your code
  ...
})();

so you do not need to add it to each method in the file

@Fedik

Fedik Jul 20, 2016

Contributor

better approach is to wrap the code in the file in to the anonymous function, see core.js for example:

!(function(){
  'use strict';
  ... 
  here is your code
  ...
})();

so you do not need to add it to each method in the file

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 21, 2016

Member

@dgt41
How would it work for non-Gregorian calendars as fa-IR (Jalali)?

Until now, some code is included in fa-IR.localise.php
class fa_IRDate extends JDate {
with a series of methods.

and the js files are overriden in media/fa-IR/js/
and installed correctly by the site language installer

<media destination="fa-IR">
        <filename>index.html</filename>
        <filename>js/index.html</filename>
        <filename>js/calendar-setup.js</filename>
        <filename>js/calendar.js</filename>
        <filename>js/calendar-setup-uncompressed.js</filename>
        <filename>js/calendar-uncompressed.js</filename>
    </media>

As their pack can be updated/installed on < joomla version in which this new feature would be merged, could we keep B/C?

I also remarked that although the calendar field is still in English when using fa-IR (vanilla.js may need an override), the dates displayed in articles back-end column and frontend info block are Jalali which means that some of the specific existing fa-IR code is still used.

Member

infograf768 commented Jul 21, 2016

@dgt41
How would it work for non-Gregorian calendars as fa-IR (Jalali)?

Until now, some code is included in fa-IR.localise.php
class fa_IRDate extends JDate {
with a series of methods.

and the js files are overriden in media/fa-IR/js/
and installed correctly by the site language installer

<media destination="fa-IR">
        <filename>index.html</filename>
        <filename>js/index.html</filename>
        <filename>js/calendar-setup.js</filename>
        <filename>js/calendar.js</filename>
        <filename>js/calendar-setup-uncompressed.js</filename>
        <filename>js/calendar-uncompressed.js</filename>
    </media>

As their pack can be updated/installed on < joomla version in which this new feature would be merged, could we keep B/C?

I also remarked that although the calendar field is still in English when using fa-IR (vanilla.js may need an override), the dates displayed in articles back-end column and frontend info block are Jalali which means that some of the specific existing fa-IR code is still used.

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 21, 2016

Member

@infograf768 I will provide a nice and way more simplified workflow for all the non-gregorian calendars. My idea was to have the calendar.js as abstract as possible to be able to cope with any calendar. The lines required for the fa-IR calendar are like 200 and that should be about any other non gregorian calendar (of course this needs to be an overridable file). This means that Hebrew, Hindi and all other strange calendars could be part of Joomla and that will be a great marketing point!
Please allow me to demonstrate the whole thing after this weekend, as it still needs to be refined...

PS: PHP side needs no override for any non-gregorian cal

Member

dgrammatiko commented Jul 21, 2016

@infograf768 I will provide a nice and way more simplified workflow for all the non-gregorian calendars. My idea was to have the calendar.js as abstract as possible to be able to cope with any calendar. The lines required for the fa-IR calendar are like 200 and that should be about any other non gregorian calendar (of course this needs to be an overridable file). This means that Hebrew, Hindi and all other strange calendars could be part of Joomla and that will be a great marketing point!
Please allow me to demonstrate the whole thing after this weekend, as it still needs to be refined...

PS: PHP side needs no override for any non-gregorian cal

@JoomliC

This comment has been minimized.

Show comment
Hide comment
@JoomliC

JoomliC Jul 21, 2016

Contributor

This means that Hebrew, Hindi and all other strange calendars could be part of Joomla and that will be a great marketing point!

@dgt41 cc/ @infograf768 :
Exactly how i would see it! (and much more easier to maintain, and prevent errors). 👍

About globalization of dates
Additional comment, about possible improvements, where translators would be a great help ;-)

  1. Order for month/year
    In some language, year is displayed before month in a calendar type, eg. Most Asian languages (eg. Japanese 2016年1月 where year is displayed before month in this case.
    Idea: <monthBeforeYear>1</monthBeforeYear> attribute in language xml install
  2. Usage of prefix/suffix/separator
    eg. Japanese 2016年1月 where usage of numerics for year and month with a fix suffix for each one
  3. Nominative/genitive grammatical cases
    Month spelling difference in some language in nominative/genitive grammatical case, eg. Russian/Ukrainian : in calendar (JANUARY) the nominative case Cічень 2014 and in a full date display, usage of genitive spelling 16 січня 2014 р.
    So, what about new strings of translation for month when used in a nominative way (calendar picker) ? cc/ @infograf768 @MATsxm

Maybe some other cases to take into account...?

Note 1: for mac user, iCal is a good way to visualize globalization of dates in calendar header, by switching system language. ;-)

I think that a list of all date formats, depending on each culture currently supported by Joomla! could be a useful starting point to see how to improve globalization ?

Note 2: i've tried to propose a PR some time ago (when i started to contribute) but this one got no real success feedback. But this could help to see the logic, and maybe find a best way to manage the calendar picker in a i18n way ? (see for details: #2809)

Contributor

JoomliC commented Jul 21, 2016

This means that Hebrew, Hindi and all other strange calendars could be part of Joomla and that will be a great marketing point!

@dgt41 cc/ @infograf768 :
Exactly how i would see it! (and much more easier to maintain, and prevent errors). 👍

About globalization of dates
Additional comment, about possible improvements, where translators would be a great help ;-)

  1. Order for month/year
    In some language, year is displayed before month in a calendar type, eg. Most Asian languages (eg. Japanese 2016年1月 where year is displayed before month in this case.
    Idea: <monthBeforeYear>1</monthBeforeYear> attribute in language xml install
  2. Usage of prefix/suffix/separator
    eg. Japanese 2016年1月 where usage of numerics for year and month with a fix suffix for each one
  3. Nominative/genitive grammatical cases
    Month spelling difference in some language in nominative/genitive grammatical case, eg. Russian/Ukrainian : in calendar (JANUARY) the nominative case Cічень 2014 and in a full date display, usage of genitive spelling 16 січня 2014 р.
    So, what about new strings of translation for month when used in a nominative way (calendar picker) ? cc/ @infograf768 @MATsxm

Maybe some other cases to take into account...?

Note 1: for mac user, iCal is a good way to visualize globalization of dates in calendar header, by switching system language. ;-)

I think that a list of all date formats, depending on each culture currently supported by Joomla! could be a useful starting point to see how to improve globalization ?

Note 2: i've tried to propose a PR some time ago (when i started to contribute) but this one got no real success feedback. But this could help to see the logic, and maybe find a best way to manage the calendar picker in a i18n way ? (see for details: #2809)

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 21, 2016

Member

Order for month/year

Will add a switch for that

Usage of prefix/suffix/separator

Will add the needed logic

Nominative/genitive grammatical cases

@JoomliC can you check if https://github.com/samsonjs/strftime is already supporting this, as I am gonna use it instead of the Date.print() custom function that misses a bunch of options

@infograf768 do you know if the date that is stored in the database for fa-IR is the converted to gregorian or is it the local, (can't check at the moment)

Member

dgrammatiko commented Jul 21, 2016

Order for month/year

Will add a switch for that

Usage of prefix/suffix/separator

Will add the needed logic

Nominative/genitive grammatical cases

@JoomliC can you check if https://github.com/samsonjs/strftime is already supporting this, as I am gonna use it instead of the Date.print() custom function that misses a bunch of options

@infograf768 do you know if the date that is stored in the database for fa-IR is the converted to gregorian or is it the local, (can't check at the moment)

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 21, 2016

Member

The date in the database is Unix format

Member

infograf768 commented Jul 21, 2016

The date in the database is Unix format

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 24, 2016

Member

@JoomliC So far I implemented only the first of the three points you mention above (year-month order of display). Before I go on and implement the other ones I would like some input from the translator team if this is something that will be approved so @infograf768 and @Bakual the ball is on your side 😄.

The status is: calendar-vanilla.js is complete with all the hooks for the date-helper.js (this needs some serious clean up).
As an example I will provide the Persian calendar date-helper.js (for B/C also)

PS. If you check the files you'll see that I touched core.js to add a call to a function for exchanging values from a data attribute instead of recalculating the input value before form submission. I hope this doesn't break B/C is some bizarre way

Member

dgrammatiko commented Jul 24, 2016

@JoomliC So far I implemented only the first of the three points you mention above (year-month order of display). Before I go on and implement the other ones I would like some input from the translator team if this is something that will be approved so @infograf768 and @Bakual the ball is on your side 😄.

The status is: calendar-vanilla.js is complete with all the hooks for the date-helper.js (this needs some serious clean up).
As an example I will provide the Persian calendar date-helper.js (for B/C also)

PS. If you check the files you'll see that I touched core.js to add a call to a function for exchanging values from a data attribute instead of recalculating the input value before form submission. I hope this doesn't break B/C is some bizarre way

media/system/js/core-uncompressed.js
+ // so there will be no reason to recalculate the values
+ // for non Gregorian calendars
+ if (typeof getJoomlaCalendarValuesFromAlt == "function") {
+ getJoomlaCalendarValuesFromAlt();

This comment has been minimized.

@Fedik

Fedik Jul 25, 2016

Contributor

sorry, but it is bad, it is very bad.
I do not very understood why do not use submit event,
in JoomlaCalendar you already know the form so you can do same on submit:

calendarinput.form.addEventListener('submit', function(){
  // here is the code from 
})
@Fedik

Fedik Jul 25, 2016

Contributor

sorry, but it is bad, it is very bad.
I do not very understood why do not use submit event,
in JoomlaCalendar you already know the form so you can do same on submit:

calendarinput.form.addEventListener('submit', function(){
  // here is the code from 
})
@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 25, 2016

Member

Are there any objections on making the field readonly?
screen shot 2016-07-25 at 19 24 07

Member

dgrammatiko commented Jul 25, 2016

Are there any objections on making the field readonly?
screen shot 2016-07-25 at 19 24 07

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Jul 25, 2016

Contributor

Are there any objections on making the field readonly?

then need to add 'Clear' button, and open the calendar also after click on the input 😉

Contributor

Fedik commented Jul 25, 2016

Are there any objections on making the field readonly?

then need to add 'Clear' button, and open the calendar also after click on the input 😉

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jul 25, 2016

Member

@Fedik I have already implemented the option to open on click in the input (but needs the button hidden, probably I should change that). I guess clear should be there only for non required fields, doable, will do it later on...

Member

dgrammatiko commented Jul 25, 2016

@Fedik I have already implemented the option to open on click in the input (but needs the button hidden, probably I should change that). I guess clear should be there only for non required fields, doable, will do it later on...

media/system/js/calendar-vanilla.js
+ var bind = function (element) {
+ if (hasClass(elem.getElementsByTagName("button")[0], 'hidden'))
+ {
+ addCalEvent(element.parentNode.getElementsByTagName('INPUT')[0], 'focus', show, true);

This comment has been minimized.

@dgrammatiko

dgrammatiko Jul 25, 2016

Member

@Fedik open on input click is already here 😉

@dgrammatiko

dgrammatiko Jul 25, 2016

Member

@Fedik open on input click is already here 😉

This comment has been minimized.

@Fedik

Fedik Jul 25, 2016

Contributor

ups 😃

@Fedik

Fedik Jul 25, 2016

Contributor

ups 😃

This comment has been minimized.

@dgrammatiko

dgrammatiko Jul 25, 2016

Member

@Fedik if you have some spare time I would appreciate a review of the code

@dgrammatiko

dgrammatiko Jul 25, 2016

Member

@Fedik if you have some spare time I would appreciate a review of the code

@laoneo

This comment has been minimized.

Show comment
Hide comment
@laoneo

laoneo Jul 26, 2016

Member

Will miss the old calendar then 🙊. Didn't had a look on the code yet, but will it be possible to specify a date format for the input field then?

Member

laoneo commented Jul 26, 2016

Will miss the old calendar then 🙊. Didn't had a look on the code yet, but will it be possible to specify a date format for the input field then?

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Jul 26, 2016

Contributor

couple notes about the pull request:

  1. date-helper.js would be good to wrap in to anonimous function and for "use strict";:
!(function(Date){
   'use strict';
   ... code of date-helper
})(Date);
  1. the filter thing (switch (strtoupper($filter))...) still need to be in the field file, it part of the logic, not the styling. If someone accidentally remove it then he will get wrong time on each save (if timezone not UTC).

  2. you use strange construction:

var JoomlaCalendar = function (selector) {
  ...
  createInstance = function (element) {
    ...
    element._joomlaCalendar = new JoomlaCalendar.init(element);
    ...
  }
  ...
};
/** The Calendar object constructor. */
JoomlaCalendar.init = function (elem) {
};

You have mixed Class definition with setup.
It looks bad, very bad 😉

Better split the thing, and all will be more easy:

var JoomlaCalendarSetup = function (selector) {
  ...
  createInstance = function (element) {
    ...
    element._joomlaCalendar = new JoomlaCalendar(element);
    ...
  }
  ...
};
/** The Calendar object constructor. */
JoomlaCalendar = function (element, options) {
};
JoomlaCalendar.updateTime = function(){...}
JoomlaCalendar.setDate = function(){...}

And why need this createInstance at all? just make element._joomlaCalendar = new JoomlaCalendar(element);

Some more will come later :neckbeard:

Contributor

Fedik commented Jul 26, 2016

couple notes about the pull request:

  1. date-helper.js would be good to wrap in to anonimous function and for "use strict";:
!(function(Date){
   'use strict';
   ... code of date-helper
})(Date);
  1. the filter thing (switch (strtoupper($filter))...) still need to be in the field file, it part of the logic, not the styling. If someone accidentally remove it then he will get wrong time on each save (if timezone not UTC).

  2. you use strange construction:

var JoomlaCalendar = function (selector) {
  ...
  createInstance = function (element) {
    ...
    element._joomlaCalendar = new JoomlaCalendar.init(element);
    ...
  }
  ...
};
/** The Calendar object constructor. */
JoomlaCalendar.init = function (elem) {
};

You have mixed Class definition with setup.
It looks bad, very bad 😉

Better split the thing, and all will be more easy:

var JoomlaCalendarSetup = function (selector) {
  ...
  createInstance = function (element) {
    ...
    element._joomlaCalendar = new JoomlaCalendar(element);
    ...
  }
  ...
};
/** The Calendar object constructor. */
JoomlaCalendar = function (element, options) {
};
JoomlaCalendar.updateTime = function(){...}
JoomlaCalendar.setDate = function(){...}

And why need this createInstance at all? just make element._joomlaCalendar = new JoomlaCalendar(element);

Some more will come later :neckbeard:

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Nov 20, 2016

Member

@roland-d clear your browser's cache

Member

dgrammatiko commented Nov 20, 2016

@roland-d clear your browser's cache

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Nov 20, 2016

Contributor

@dgt41 Cleared cache, used inPrivate window still the same result. So I setup a new VM to make sure all is clear but even then it happens. Anything I can/should check to make sure I have the up-to-date version of the code? Now it has been applied using Patchtester as a direct git apply won't work.

Contributor

roland-d commented Nov 20, 2016

@dgt41 Cleared cache, used inPrivate window still the same result. So I setup a new VM to make sure all is clear but even then it happens. Anything I can/should check to make sure I have the up-to-date version of the code? Now it has been applied using Patchtester as a direct git apply won't work.

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Nov 20, 2016

Member

@roland-d did I ever mention that I am a big fan of IE?
So it seems that execution order matters thus I moved the code to show function. This should guaranty that chosen never messes with the calendar's select elements

screen shot 2016-11-20 at 14 54 47

Member

dgrammatiko commented Nov 20, 2016

@roland-d did I ever mention that I am a big fan of IE?
So it seems that execution order matters thus I moved the code to show function. This should guaranty that chosen never messes with the calendar's select elements

screen shot 2016-11-20 at 14 54 47

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Nov 20, 2016

Member

@roland-d can you confirm that IE8 is good after the last commit?

Member

dgrammatiko commented Nov 20, 2016

@roland-d can you confirm that IE8 is good after the last commit?

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Nov 20, 2016

Contributor

@dgt41 Working fine here with the latest fixes. 👍

Contributor

roland-d commented Nov 20, 2016

@dgt41 Working fine here with the latest fixes. 👍

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Nov 20, 2016

Member

@roland-d let's RTC this then

Member

dgrammatiko commented Nov 20, 2016

@roland-d let's RTC this then

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Nov 21, 2016

Contributor

Set to RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

Contributor

roland-d commented Nov 21, 2016

Set to RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

@joomla-cms-bot joomla-cms-bot added the RTC label Nov 21, 2016

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Nov 21, 2016

Contributor

Sorry but I am removing RTC for now

In the Beez3 and hathor templates the cursor is an ibeam on the clear, today, close links when it should be a pointer


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.
Contributor

brianteeman commented Nov 21, 2016

Sorry but I am removing RTC for now

In the Beez3 and hathor templates the cursor is an ibeam on the clear, today, close links when it should be a pointer


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

@joomla-cms-bot joomla-cms-bot removed the RTC label Nov 21, 2016

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Nov 21, 2016

Member

I've patched the css so hathor and beez look like:
screen shot 2016-11-21 at 12 24 34
screen shot 2016-11-21 at 12 24 06

@brianteeman once this is merged we can call @C-Lodder and @ciar4n to do some more styling here

Member

dgrammatiko commented Nov 21, 2016

I've patched the css so hathor and beez look like:
screen shot 2016-11-21 at 12 24 34
screen shot 2016-11-21 at 12 24 06

@brianteeman once this is merged we can call @C-Lodder and @ciar4n to do some more styling here

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Nov 21, 2016

Contributor

From the screenshot I cant see if you have addressed the issue I raised. The CURSOR

Contributor

brianteeman commented Nov 21, 2016

From the screenshot I cant see if you have addressed the issue I raised. The CURSOR

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Nov 21, 2016

Member

check the code: cc7e8af

Member

dgrammatiko commented Nov 21, 2016

check the code: cc7e8af

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Nov 21, 2016

Member

Pointers OK now

pointer

Member

infograf768 commented Nov 21, 2016

Pointers OK now

pointer

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Nov 21, 2016

Member

Back to RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

Member

infograf768 commented Nov 21, 2016

Back to RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

@joomla-cms-bot joomla-cms-bot added the RTC label Nov 21, 2016

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Nov 21, 2016

Contributor

Thanks

On 21 November 2016 at 10:47, infograf768 notifications@github.com wrote:

Back to RTC

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/tracker/
joomla-cms/11138.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11138 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8Yn-p74iP40tuE6XwCHWV4NsiZbQks5rAXargaJpZM4JM8Zz
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

Contributor

brianteeman commented Nov 21, 2016

Thanks

On 21 November 2016 at 10:47, infograf768 notifications@github.com wrote:

Back to RTC

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/tracker/
joomla-cms/11138.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11138 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8Yn-p74iP40tuE6XwCHWV4NsiZbQks5rAXargaJpZM4JM8Zz
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

@wilsonge wilsonge merged commit 0074a39 into joomla:staging Nov 21, 2016

2 checks passed

continuous-integration/drone the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wilsonge

This comment has been minimized.

Show comment
Hide comment
@wilsonge

wilsonge Nov 21, 2016

Contributor

Merged

Contributor

wilsonge commented Nov 21, 2016

Merged

@Fedik

This comment has been minimized.

Show comment
Hide comment
@Fedik

Fedik Nov 21, 2016

Contributor

🎉 🎊🎉 🎊🎉 🎊

Contributor

Fedik commented Nov 21, 2016

🎉 🎊🎉 🎊🎉 🎊

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Nov 22, 2016

Member

@dgt41
$translateFormat is no more in the calendar.php form and therefore does not work.
We still have the strings.

Member

infograf768 commented Nov 22, 2016

@dgt41
$translateFormat is no more in the calendar.php form and therefore does not work.
We still have the strings.

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Nov 22, 2016

Contributor

Sounds like a wrong merge conflict solving to me.

I can readd it.

Contributor

Bakual commented Nov 22, 2016

Sounds like a wrong merge conflict solving to me.

I can readd it.

roland-d added a commit to roland-d/joomla-cms that referenced this pull request Nov 25, 2016

Merge branch 'staging' into shareable-draft-content-feature
* staging: (98 commits)
  Coding style. PHP constants true, false, and null MUST be in lower case. (#13010)
  Removing duplicated AS in sql query (#13006)
  Fixed typo in comment (#12992)
  Correcting strings in TFA Google plugin (#12980)
  code style changes (#12986)
  Error in sr-YU installation ini file (#12984)
  New DateTime picker (replaces calendar) (#11138)
  Export of Banners Tracks Does Not Export the Banner Name
  fix rues get data (#12763)
  Added Feature items filter to mod_articles_news (#12547)
  fix them all (#12943)
  a11y regression fix (#12935)
  Set correct component id for system links (#12938)
  Fix for Undefined offset in Content History preview popup (#12791)
  remove tab on meta charset (#12895)
  JSession patched to set session _state to 'inactive' when session is closed. (#12928)
  [JHtmlNumber::bytes] Format number according to language (#12929)
  Update edit.php (#12818)
  Update default.xml (#12917)
  Adding the ability to use the global value for character count in newsfeeds (#12869)
  ...

@dgrammatiko dgrammatiko deleted the dgrammatiko:+FieldCalendarNew branch Mar 18, 2017

@Jeldik

This comment has been minimized.

Show comment
Hide comment
@Jeldik

Jeldik Oct 9, 2017

When I use date format %d.%m and save it, Joomla returns me date format in reverse result(%m.%d) in my view. I am fetching data in good format from my DB. When I debug this, I can see that problem is in Calendar.js -> checkInputs -> here DateHelper.js returns me wrong format. I am sending for example 7.9. (d.m) and I get 9.7.

Can someone check it, please?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

Jeldik commented Oct 9, 2017

When I use date format %d.%m and save it, Joomla returns me date format in reverse result(%m.%d) in my view. I am fetching data in good format from my DB. When I debug this, I can see that problem is in Calendar.js -> checkInputs -> here DateHelper.js returns me wrong format. I am sending for example 7.9. (d.m) and I get 9.7.

Can someone check it, please?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Oct 9, 2017

@Jeldik can you please open a new Issue?

@Jeldik can you please open a new Issue?

@Jeldik

This comment has been minimized.

Show comment
Hide comment

Jeldik commented Oct 10, 2017

@franz-wohlkoenig Done

@dodinz

This comment has been minimized.

Show comment
Hide comment
@dodinz

dodinz Jun 14, 2018

Is it possible to use only the time picker without the dates?

dodinz commented Jun 14, 2018

Is it possible to use only the time picker without the dates?

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Jun 14, 2018

Contributor

@dodinz This PR has been merged. If you have further questions, please open a new issue.

Contributor

Bakual commented Jun 14, 2018

@dodinz This PR has been merged. If you have further questions, please open a new issue.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 14, 2018

@Bakual dodinz has opened #20747 two Hours ago.

@Bakual dodinz has opened #20747 two Hours ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment