-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Conversation
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? |
Does this work with the folowing config options from the language xml
|
@brianteeman I hope so:
@infograf768 Not yet, (rtl is tested tho). As I said #707 has all the changes needed, so it should be fairly easy to do |
@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. |
@JoomliC thanks! Seems I forgot to uncomment these lines: https://github.com/dgt41/joomla-cms/blob/02c9cef6b29f44eb8dba5a1077bfa3a8b81214d8/media/system/js/calendar-vanilla.js#L575-L578 |
@JoomliC so my calculations are off. It seems I hit that often lately 😄 |
@dgt41 IMO, maybe set its position to top, and bottom only if not enought space at top ? (could be easier to manage too ;-) ) |
@infograf768 Jalali is almost ready: |
} | ||
|
||
// If a known filter is given use it. | ||
switch (strtoupper($filter)) |
There was a problem hiding this comment.
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.
@dgt41 great job! |
@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 😉 |
@dgt41 yes, this looks better, thanks! |
************************** Initialize ******************************* | ||
*********************************************************************/ | ||
document.onreadystatechange = function () { | ||
if (document.readyState == "interactive") { |
There was a problem hiding this comment.
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
...
I have tested this item ✅ successfully on 44589ad This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11138. |
@dgt41 it looks like we have merge conflicts. |
@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) In the same time, a question: |
@zero-24 these are unit tests that are failing |
@JoomliC are you spying on my computer? 😄 |
@dgt41 you didn't know i did ? 😆 |
* @license GNU General Public License version 2 or later; see LICENSE.txt | ||
*/ | ||
var JoomlaCalendar = function (selector) { | ||
'use strict'; |
There was a problem hiding this comment.
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
@dgt41 Until now, some code is included in fa-IR.localise.php and the js files are overriden in media/fa-IR/js/
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. |
@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! PS: PHP side needs no override for any non-gregorian cal |
@dgt41 cc/ @infograf768 : About globalization of dates
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) |
Will add a switch for that
Will add the needed logic
@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) |
Set to RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138. |
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. |
I've patched the css so hathor and beez look like: @brianteeman once this is merged we can call @C-Lodder and @ciar4n to do some more styling here |
From the screenshot I cant see if you have addressed the issue I raised. The CURSOR |
check the code: cc7e8af |
Back to RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11138. |
Thanks On 21 November 2016 at 10:47, infograf768 notifications@github.com wrote:
Brian Teeman |
Merged |
🎉 🎊🎉 🎊🎉 🎊 |
@dgt41 |
Sounds like a wrong merge conflict solving to me. I can readd it. |
* 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) ...
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 can you please open a new Issue? |
@franz-wohlkoenig Done |
Is it possible to use only the time picker without the dates? |
@dodinz This PR has been merged. If you have further questions, please open a new issue. |
UX UI improvements
Summary of Changes
Preview
With time picker
Basic view
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
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