Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New DateTime picker (replaces calendar) #11138

Merged
merged 139 commits into from Nov 21, 2016
Merged

New DateTime picker (replaces calendar) #11138

merged 139 commits into from Nov 21, 2016

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko 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 bootstrap calendar - with fa-ir lang and default template override css f... #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
Copy link
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?

@brianteeman
Copy link
Contributor

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

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

@dgrammatiko
Copy link
Contributor Author

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

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

@cyrezdev
Copy link
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

@dgrammatiko
Copy link
Contributor Author

@cyrezdev
Copy link
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

@dgrammatiko
Copy link
Contributor Author

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

@cyrezdev
Copy link
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 ;-)

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Jul 16, 2016
@dgrammatiko
Copy link
Contributor Author

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

@dgrammatiko
Copy link
Contributor Author

@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))
Copy link
Member

@Fedik Fedik Jul 16, 2016

Choose a reason for hiding this comment

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

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

Fedik commented Jul 16, 2016

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

@dgrammatiko
Copy link
Contributor Author

@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
Copy link
Member

Fedik commented Jul 17, 2016

@dgt41 yes, this looks better, thanks!

************************** Initialize *******************************
*********************************************************************/
document.onreadystatechange = function () {
if (document.readyState == "interactive") {
Copy link
Member

Choose a reason for hiding this comment

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

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

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

zero-24 commented Jul 19, 2016

@dgt41 it looks like we have merge conflicts.

@cyrezdev
Copy link
Contributor

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

@zero-24 these are unit tests that are failing

@dgrammatiko
Copy link
Contributor Author

@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

@cyrezdev
Copy link
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 ;-)

* @license GNU General Public License version 2 or later; see LICENSE.txt
*/
var JoomlaCalendar = function (selector) {
'use strict';
Copy link
Member

@Fedik Fedik Jul 20, 2016

Choose a reason for hiding this comment

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

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

@dgrammatiko
Copy link
Contributor Author

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

@cyrezdev
Copy link
Contributor

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

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)

@roland-d
Copy link
Contributor

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 This Pull Request is Ready To Commit label Nov 21, 2016
@brianteeman
Copy link
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.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 21, 2016
@dgrammatiko
Copy link
Contributor Author

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

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

@dgrammatiko
Copy link
Contributor Author

check the code: cc7e8af

@infograf768
Copy link
Member

Pointers OK now

pointer

@infograf768
Copy link
Member

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 This Pull Request is Ready To Commit label Nov 21, 2016
@brianteeman
Copy link
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/

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

Merged

@joomla-cms-bot joomla-cms-bot added Unit/System Tests and removed RTC This Pull Request is Ready To Commit labels Nov 21, 2016
@Fedik
Copy link
Member

Fedik commented Nov 21, 2016

🎉 🎊🎉 🎊🎉 🎊

@infograf768
Copy link
Member

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

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

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.

@ghost
Copy link

ghost commented Oct 9, 2017

@Jeldik can you please open a new Issue?

@Jeldik
Copy link

Jeldik commented Oct 10, 2017

@franz-wohlkoenig Done

@dodinz
Copy link

dodinz commented Jun 14, 2018

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

@Bakual
Copy link
Contributor

Bakual commented Jun 14, 2018

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

@ghost
Copy link

ghost commented Jun 14, 2018

@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet