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

[CODE STYLING] Fixes code styling in cms-uncompressed.js #16947

Merged
merged 1 commit into from
Jul 6, 2017
Merged

[CODE STYLING] Fixes code styling in cms-uncompressed.js #16947

merged 1 commit into from
Jul 6, 2017

Conversation

regularlabs
Copy link
Contributor

@regularlabs regularlabs commented Jul 3, 2017

Fixes code styling in cms-uncompressed.js

  • indentation
  • use of braces
  • whitespace
  • etc

container = container || document;

var $showonFields = $(container).find('[data-showon]');

// Setup each 'showon' field
for (var is = 0, ls = $showonFields.length; is < ls; is++) {
// Use anonymous function to capture arguments
(function () {
(function() {

Choose a reason for hiding this comment

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

Don't make functions within a loop.

showfield = false;
}
// OR operator: one of the previous and current conditions must be valid
if (condition['op'] === 'OR' && condition['valid'] + jsondata[j-1]['valid'] > 0)
{
if (condition['op'] === 'OR' && condition['valid'] + jsondata[j - 1]['valid'] > 0) {

Choose a reason for hiding this comment

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

['op'] is better written in dot notation.
['valid'] is better written in dot notation.

// AND operator: both the previous and current conditions must be valid
if (condition['op'] === 'AND' && condition['valid'] + jsondata[j-1]['valid'] < 2)
{
if (condition['op'] === 'AND' && condition['valid'] + jsondata[j - 1]['valid'] < 2) {

Choose a reason for hiding this comment

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

['op'] is better written in dot notation.
['valid'] is better written in dot notation.

if (condition['valid'] === 0)
{
if (condition['op'] === '') {
if (condition['valid'] === 0) {

Choose a reason for hiding this comment

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

['valid'] is better written in dot notation.

{
if (condition['valid'] === 0)
{
if (condition['op'] === '') {

Choose a reason for hiding this comment

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

['op'] is better written in dot notation.

jsondata[j]['valid'] = 1;
}
// "!:" Not equal to one or more of the values condition
if (jsondata[j]['sign'] == '!=' && jsondata[j]['values'].indexOf(itemval[i]) === -1)
{
if (jsondata[j]['sign'] == '!=' && jsondata[j]['values'].indexOf(itemval[i]) === -1) {

Choose a reason for hiding this comment

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

['sign'] is better written in dot notation.
['values'] is better written in dot notation.

// ":" Equal to one or more of the values condition
if (jsondata[j]['sign'] == '=' && jsondata[j]['values'].indexOf(itemval[i]) !== -1)
{
if (jsondata[j]['sign'] == '=' && jsondata[j]['values'].indexOf(itemval[i]) !== -1) {

Choose a reason for hiding this comment

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

['sign'] is better written in dot notation.
['values'] is better written in dot notation.

}

// Convert to array to allow multiple values in the field (e.g. type=list multiple)
// and normalize as string
if (!(typeof itemval === 'object'))
{
if (!(typeof itemval === 'object')) {

Choose a reason for hiding this comment

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

Confusing use of '!'.

itemval = [];
}
// a multi-select <select> $field will return null when no elements are selected so we need to define itemval accordingly
if (itemval == null && $field.prop("tagName").toLowerCase() == "select") {

Choose a reason for hiding this comment

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

Use '===' to compare with 'null'.

@wilsonge wilsonge added this to the Joomla 3.7.4 milestone Jul 3, 2017
@wilsonge
Copy link
Contributor

wilsonge commented Jul 3, 2017

RTC on review for 3.7.4


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.4 milestone Jul 3, 2017
@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 3, 2017
@laoneo
Copy link
Member

laoneo commented Jul 3, 2017

Are the comments of the Hound CI valid ones or will they be ignored?

@wilsonge
Copy link
Contributor

wilsonge commented Jul 3, 2017

None of the issues are introduced with this PR. They're all existing so for the purposes of this PR it's all good. In the long run it should be cleaned up though

@regularlabs
Copy link
Contributor Author

I'll do a cleanup of some of these (dot notation, confusing '!') after this PR is merged.

@mbabker
Copy link
Contributor

mbabker commented Jul 3, 2017

@C-Lodder do you think you could do a Hound configuration for 3.x since it seems that's running on all PRs and not just 4.0?

@C-Lodder
Copy link
Member

C-Lodder commented Jul 3, 2017

Sure will do

@C-Lodder
Copy link
Member

C-Lodder commented Jul 3, 2017

#16956 @mbabker

@rdeutz
Copy link
Contributor

rdeutz commented Jul 6, 2017

This needs a test

@rdeutz rdeutz added PR-staging and removed RTC This Pull Request is Ready To Commit labels Jul 6, 2017
@rdeutz rdeutz added this to the Joomla 3.7.4 milestone Jul 6, 2017
@regularlabs
Copy link
Contributor Author

Code styling fixes don't need tests.
Especially not when the final minified js file (the one that gets loaded) is not affected.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 6, 2017
@rdeutz rdeutz merged commit 4f94056 into joomla:staging Jul 6, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 6, 2017
@rdeutz
Copy link
Contributor

rdeutz commented Jul 6, 2017

So it is only cosmetic ;-) Merged

@regularlabs regularlabs deleted the patch-2 branch July 6, 2017 10:49
GeraintEdwards added a commit to GeraintEdwards/joomla-cms that referenced this pull request Jul 21, 2017
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

9 participants