? ? Pending

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
6 Mar 2017

Pull Request for Issue #14338.

Summary of Changes

Overrides the default value field with a calendar field.
image

Also the tooltip has changed to better reflect the default value.

Testing Instructions

Create a calendar custom field.

Expected result

The Joomla date picker is shown as default field.

Actual result

A text area is shown as default value field.

Documentation Changes Required

The explanation of the default field must be changed.

avatar laoneo laoneo - open - 6 Mar 2017
avatar laoneo laoneo - change - 6 Mar 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 6 Mar 2017
Category Administration Language & Strings Front End Plugins
avatar laoneo laoneo - change - 6 Mar 2017
The description was changed
avatar laoneo laoneo - edited - 6 Mar 2017
avatar franz-wohlkoenig franz-wohlkoenig - test_item - 6 Mar 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 6 Mar 2017

I have tested this item successfully on 4ec63c6


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

avatar laoneo laoneo - change - 6 Mar 2017
Labels Added: ? ?
avatar laoneo
laoneo - comment - 6 Mar 2017

@dgt41 done.

@zero-24 correctly like that?

avatar Bakual
Bakual - comment - 6 Mar 2017

The default value field must allow to enter "NOW" as a valid value. It would be the most useful default value actually (it will be replaced with current date when the form is shown) ?

So either we leave it as text or we find a way to allow NOW as valid input. The latter would need a plugin specific formfield for the default value.

avatar laoneo
laoneo - comment - 6 Mar 2017

When the default value is empty then it defaults to now anyway, so it is not needed to have support for the now parameter explicitly.

avatar Bakual
Bakual - comment - 6 Mar 2017

When the default value is empty then it defaults to now anyway, so it is not needed to have support for the now parameter explicitly.

That's not true (and would be wrong anyway). When the default value is empty it defaults to empty as expected.

avatar dgt41
dgt41 - comment - 6 Mar 2017

Use pattern="Some regex" to customize the validator in client side

avatar laoneo
laoneo - comment - 6 Mar 2017

@Bakual not the value in the field, but when you open the editor, then it does.

avatar zero-24
zero-24 - comment - 6 Mar 2017

@laoneo fixed with c2e0ea2

@laoneo is it correct that the new field is outside of the fieldset?

avatar laoneo
laoneo - comment - 6 Mar 2017

@zero-24 yes it is correct, because it overwrites the default field of the normal form and not adding an additional parameter.

avatar zero-24
zero-24 - comment - 6 Mar 2017

? I just want to be sure ;)

avatar Bakual
Bakual - comment - 6 Mar 2017

not the value in the field, but when you open the editor, then it does.

Yep, when you open the calendar popup, it will start with the current date. But that's not exactly a default value (which would prevent saving an empty value).

avatar laoneo
laoneo - comment - 7 Mar 2017

So what do you suggest then @Bakual? There is no way to put a string into the datepicker field. What can be done is when the field is required to default it with the current date from the PHP side. Like that we have the "Now" behavior.

avatar Bakual
Bakual - comment - 7 Mar 2017

I would try an adjusted field where you can toggle between NOW and datepicker mode.

avatar laoneo
laoneo - comment - 8 Mar 2017

What you guys think, should we go with this PR and don't have support for the "NOW" parameter or should we leave it as a free textarea? I honestly don't have an opinion at all.

avatar Bakual
Bakual - comment - 8 Mar 2017

I had a look and imho creating a field which allows to enter both is to complex - at least for me.

Personally I wouldn't use the popup to enter the default value and keep the possibility for a NOW value. Which currently means leaving it as freetext. I weight the possibility for a "NOW" value higher since that is a quite useful default value.
However I think it would be great to adjust the tooltip (description) of this field so it is clear that the value needs to be entered in the english format (YYYY-MM-DD HH:MM:SS) and that it allows "NOW" as value.
Overriding the tooltip would be simple. @laoneo already did that in another PR for another custom field ?

avatar dgt41
dgt41 - comment - 8 Mar 2017

@Bakual using a custom element here you could do that in less than 50 lines, but we don't have custom elements...

avatar joomla-cms-bot joomla-cms-bot - change - 9 Mar 2017
Category Administration Language & Strings Front End Plugins Administration Language & Strings
avatar laoneo
laoneo - comment - 9 Mar 2017

Reverted the default value and set a better default text.

avatar Bakual
Bakual - comment - 9 Mar 2017

using a custom element here you could do that in less than 50 lines, but we don't have custom elements...

@dgt41 Actually, we could do what you would do in your custom element just fine with an adjusted plugin specific formfield here. That's not the issue. I just lack the skill to create the JS needed for that and it would take me far more time than it is worth imho. If you know how to do it and make it look pretty, we could do it without any core changes.

avatar dgt41
dgt41 - comment - 9 Mar 2017

@Bakual the js for switching between the 2 values should be exactly the same as #13113

avatar Bakual
Bakual - comment - 10 Mar 2017

Nah, that wont work because if I understand it right it just deletes the value of another field on submit, but here we need the same field (default_value) to be able to store either "NOW" or a date (with picker).

Ideally the calendar field would just have an added button "NOW" and if enabled it submits "NOW" for the field.
Another thing I would have imagined is a switcher which toggles between two states a "NOW" and the calendar. The tricky part is that both will have to send the same "name" of the field. I assumed if we use two fields with the same name and the inactive is disabled and thus will not be sent it could work. But haven't tested.

Anyway, I lack the skills to do it.

avatar brianteeman
brianteeman - comment - 25 Mar 2017

Where are we at with this one?

avatar Bakual
Bakual - comment - 25 Mar 2017

The proposed solution imho is wrong as it limits the useage. The existing text field isn't "nice" but doesn't limit anything.
Nos other solution coded.

avatar laoneo
laoneo - comment - 25 Mar 2017

I'v changed the implementation that only the default label and tooltip are changed, not the field type itself. But it needs to wait till #14793 get merged.

avatar laoneo
laoneo - comment - 27 Mar 2017

This pr is overriding the default strings now correctly, it doesn't change the field type of the default field:
image

I guess like that we can give it a go for testing.

avatar Bakual Bakual - change - 28 Mar 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-03-28 09:19:15
Closed_By Bakual
avatar Bakual Bakual - close - 28 Mar 2017
avatar Bakual Bakual - merge - 28 Mar 2017
avatar Bakual
Bakual - comment - 28 Mar 2017

Merged

avatar normanmm normanmm - test_item - 28 Mar 2017 - Tested unsuccessfully
avatar normanmm
normanmm - comment - 28 Mar 2017

I have tested this item ? unsuccessfully on 03fe667

The default value is displayed in the front end but only the date part, the time is not displayed. Thus, for instance the value 2017-08-24 13:45:01 is displayed as 2017-08-24.
Another thing: are the default values set for the field at the article level (Articles:Edit --> Fields tab) override the original field values?(Articled:Edit Field). They don't.


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

avatar normanmm normanmm - test_item - 28 Mar 2017 - Tested successfully
avatar normanmm
normanmm - comment - 28 Mar 2017

I have tested this item successfully on 03fe667

Forget my previous test.I had not set the Show Time option to YES


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

Add a Comment

Login with GitHub to post a comment