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

Number validation as global handler #9998

Closed
wants to merge 3 commits into from

Conversation

jackkum
Copy link
Contributor

@jackkum jackkum commented Apr 20, 2016

Pull Request for Issue #5619 .

Summary of Changes

  • Added handler for the number field validation
  • Minified uncompressed file

Testing Instructions

  1. Go to the Joomla administration
  2. Then: System => Global Configuration => System (Tab) => Session Settings
  3. Try input in the field 'Session Lifetime' zero or negative value

You should see error (red border around and title on mouseover)

This PR add handler as global, only for session lifetime (com_config) here: #9997

@piotr-cz
Copy link
Contributor

Could you use 4 spaces as described here?

This file is really botched in terms of indentation
tabsspacesboth

@jackkum
Copy link
Contributor Author

jackkum commented Apr 20, 2016

@piotr-cz 😆
Fix all?

@piotr-cz
Copy link
Contributor

piotr-cz commented Apr 20, 2016

I'd love to, but probably not in this PR, because it would break git diffs (all lines would show as changed).

Looks like 2x tabs or eventually 8x space would fit current CS here (ATM there is space + tab + space + tab 😄 ).

@Fedik
Copy link
Member

Fedik commented Apr 20, 2016

Hm, why it names "number" validation? I see only some "range" validation.
It will return true for the string that not a number.

@jackkum
Copy link
Contributor Author

jackkum commented Apr 21, 2016

Need a PR for add class="validate-range" in a file administrator/components/com_config/model/form/application.xml for the field lifetime.

@brianteeman
Copy link
Contributor

@jackkum Sorry I dont understand your last comment


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

@jackkum
Copy link
Contributor Author

jackkum commented May 8, 2016

@brianteeman
By @Fedik advice i renamed number to range and validation on client side for the field lifetime will not be applied, so need a PR for this.

Sorry for my English.

@tomartailored
Copy link

Can you please mention the Both scenario before patch and after patch, as i tested and it seems working same in both cases


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

@RonakParmar
Copy link

I have tested this PR in Joomla! 3.6.4, before patch and after patch the session field is working same in bath scenarios.


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

@jeckodevelopment
Copy link
Member

@jackkum can you please fix the conflicts?
media/system/js/validate.js

@ghost
Copy link

ghost commented Jan 6, 2017

I have tested this item ✅ successfully on d2a18fb

Before and after Patch works in same Way.


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

@RonakParmar
Copy link

@jackkum Can you please fix conflict in media/system/js/validate.js file?

@brianteeman
Copy link
Contributor

@jackkum it has been six months since the first request to resolve the merge conflicts. Also as several people have reported that they cannot see any difference after applying the PR I am going to close this at this time. It can always be reopened if you resolve the conflicts and address the comments about no change after this PR

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.

None yet

8 participants