? Pending

User tests: Successful: Unsuccessful:

avatar C-Lodder
C-Lodder
4 Nov 2019

Pull Request for Issue # .

Summary of Changes

When a calendar field is readonly or disabled, the button next to the input to open the calendar popup is not needed. Currently, a hidden class is added to the button to make it invisible, but this PR removes it from the DOM....seeing as it's not required at all.

Testing Instructions

  1. Go to create a new article
  2. Go to the "Publishing" tab
  3. Look at the "Modified Date" field

There shouldn't be a button next to the input field and the markup for it shouldn't be part of the DOM.

avatar C-Lodder C-Lodder - open - 4 Nov 2019
avatar C-Lodder C-Lodder - change - 4 Nov 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Nov 2019
Category Layout
avatar C-Lodder C-Lodder - change - 4 Nov 2019
Labels Added: ?
avatar Quy Quy - test_item - 4 Nov 2019 - Tested successfully
avatar Quy
Quy - comment - 4 Nov 2019

I have tested this item successfully on 5f7136b


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

avatar richard67 richard67 - test_item - 5 Nov 2019 - Tested unsuccessfully
avatar richard67
richard67 - comment - 5 Nov 2019

I have tested this item ? unsuccessfully on 5f7136b

With this PR applied, I get a JS error on browser console: "TypeError: s is undefined" in file calendar.min.js when opening an article for edit.

With debug on so unminified script is used: TypeError: btn is undefined in file calendar.js, line 72, column 5.

Without this PR applied, I don't get this error.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26980.
avatar dgrammatiko
dgrammatiko - comment - 5 Nov 2019

If you remove the button then the whole calendar script will not applied to the field. This is particularly problematic for some cases. In sort the js is initialized through the button element, so it cannot be removed.

That said, what is your use case, maybe there is another workaround...

avatar C-Lodder
C-Lodder - comment - 6 Nov 2019
  1. If you dont require an element, dont add it to the DOM
  2. Due to the nature of Joomla's classitis, if one creates a new template (both front or backend) then they need to add all these pointless classes because they're used in core. And I'm talking about those who care about their sanity and do not wish to override everything.

As for the JS, there is nothing to initialise on readonly or disabled calendar fields. The calendar should not open, period. Unless you're telling me that the JS manipulate the value on page load.

avatar dgrammatiko
dgrammatiko - comment - 6 Nov 2019

As for the JS, there is nothing to initialise on readonly or disabled calendar fields.

Disabled calendar fields doesnt mean that the JS part doesnt run (it does, check with some multilingual site) and also the calendar doesnt open but the parsing/reformatting NEEDS to happen...

avatar C-Lodder
C-Lodder - comment - 6 Nov 2019

and also the calendar doesnt open but the parsing/reformatting NEEDS to happen

So the requirement of a button being part of the DOM to initialise JS is just wrong. Buttons are clickable elements. I'd strongly suggest using type=date or data-dalendar to init the JS for value manipulation.

avatar dgrammatiko
dgrammatiko - comment - 6 Nov 2019

So the requirement of a button being part of the DOM to initialise JS is just wrong.

Right or wrong, this is what's there, you can't remove it for purity or you're just breaking the calendar, or even if you fix that you still breaking B/C...

FWIW the calendar needs to move to custom elements as the rest of the form fields. I've said that countless times...

avatar C-Lodder
C-Lodder - comment - 6 Nov 2019

I can't see how moving the JS init based on the input rather than the button would break B/C. I respect the fact that this PR may break the calendar and am happy to close, but a button requirement when it's not even visible is just stupid.

avatar dgrammatiko
dgrammatiko - comment - 6 Nov 2019

Then do that, but this can't fly just by removing the button, there's much more there. Thats what I pointed out here

EDIT:
Then do that

moving the JS init based on the input rather than the button

avatar C-Lodder C-Lodder - change - 8 Nov 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-11-08 15:05:50
Closed_By C-Lodder
avatar C-Lodder C-Lodder - close - 8 Nov 2019

Add a Comment

Login with GitHub to post a comment