Skip to content

[#17744] - Pressing clear button of calendar on required date field submits admin form in Joomla! 3.7.5 --- corrected#17793

Closed
akritianand wants to merge 10 commits intojoomla:stagingfrom
akritianand:staging
Closed

[#17744] - Pressing clear button of calendar on required date field submits admin form in Joomla! 3.7.5 --- corrected#17793
akritianand wants to merge 10 commits intojoomla:stagingfrom
akritianand:staging

Conversation

@akritianand
Copy link
Copy Markdown
Contributor

@akritianand akritianand commented Aug 31, 2017

Pull Request for Issue # 17744

Summary of Changes

Previously, when we edit an article under publishing tab, the clear button in calendar field did not work if it was a required field.
Now, it clears the calendar field when pressed instead of redirecting (as it did originally).
Changes were made in calendar.js and calendar.php

Testing Instructions

Create an admin form with a required date field. i.e. open administrator\components\com_content\models\forms\article.xml and make field "publish_down" required by adding required="true" to field's xml node.
Go to Joomla! administration panel and open an article edit view.
Finish Publishing field should now be a required field.
Add Finish Publishing date, using the date picker of the calendar
Click "Clear" button on the Calendar.

[#17729] - Color of the error message after attempting to delete a Admin-menu - SOLVED
Changed 'alert-success' to 'alert-danger' in line 14
Corrected the functionality of clear button in the finish publishing date in the article setting.
changed calender.min.js to calendar.js as this is where I have made changes.
if (!this.inputField.hasAttribute('required')) {
var savea = row.querySelector('[data-action="clear"]');
savea.addEventListener("click", function (e) {
if (1) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we don't need to check for the required attribute? And why if (1)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@akritianand IMHO, the clear button should be hidden when field is required.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, if that is required, then there was not any problem initially. Because the clear button still showed up if the field was required; it just did not perform its function.

@izharaazmi
Copy link
Copy Markdown
Contributor

@akritianand You need to generate the calendar.min.js too.

JHtml::_('script', $localesPath, false, true, false, false, true);
JHtml::_('script', $helperPath, false, true, false, false, true);
JHtml::_('script', 'system/fields/calendar.min.js', false, true, false, false, true);
JHtml::_('script', 'system/fields/calendar.js', false, true, false, false, true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please revert this change and update the calendar.min.js file with your changes

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Aug 31, 2017

Also needs updating to deconflict with the now merged #17777

@ghost
Copy link
Copy Markdown

ghost commented Sep 2, 2017

similar #17809


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

@ghost ghost mentioned this pull request Sep 2, 2017
@izharaazmi
Copy link
Copy Markdown
Contributor

This pr is broken and we have #17809 for the same issue. This should be closed, imho.

@joomla-cms-bot
Copy link
Copy Markdown

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/17793

@ghost
Copy link
Copy Markdown

ghost commented Sep 3, 2017

closed as stated above.


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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants