It seems we could use some specific examples and guidelines for jQuery coding standards.

Here are some rough suggestions for examples we could provide:

1. Indent a function definition, EG:

fieldset.bind('summaryUpdated', function () {
  var text = $.trim(fieldset.getSummary());
  summary.html(text ? ' (' + text + ')' : '');
})

2. Keep jQuery calls at the same level when they act sequentially on the same object, EG:

.append(fieldset.hasClass('collapsed') ? Drupal.t('Show') : Drupal.t('Hide'))
.after(' ')
.andSoOn()
.andSoOn()

3. Indent lines when we use a new variable to start a chain of actions on a different object. In this example, the .append applies to the div, not the same element the .after applies to:

.after($('<div class="fieldset-wrapper"></div>')
  .append(fieldset.children(':not(legend):not(.action)'))
);

4. Indent when .find() starts a chain of actions on a different object, EG:

.removeClass('collapsed')
.find('> legend > a > span.element-invisible')
  .empty()
  .append(Drupal.t('Hide'));

5. Do not end a line with ".", instead start "." on a new line.
Do not do this BAD thing:

fieldset.
  bind(...

Instead do this GOOD thing:

fieldset
  .bind(...

6. Close parentheses and brackets at the same indent level as the start of the line where they were opened. Stack them when we close two or more that start at the same depth. EG, the closing parens are on different lines but the last line has a closing bracket and paren together because they were both started together:

$('fieldset.collapsible > legend', context).once('collapse', function () {
  $(this).empty().append($('<a href="#">' + text + '</a>')
    .prepend($('<span class="element-invisible"></span>')
      .append(fieldset.hasClass('collapsed') ? Drupal.t('Show') : Drupal.t('Hide'))
      .after(' ')
    )
  )
});

7. It is okay to stack repeated/related jQuery calls on the same line, EG $(this).empty().append in the above snippet.

This is all just an idea to start the discussion.

This was prompted by conversations in the issue queue and irc with @tha_sun about misc/collapse.js in #541568: Link to expand / collapse fieldsets has poorly accessible link text. The existing indents and formatting were inconsistent in collapse.js and presumably elsewhere.

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

subscribing

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +Coding standards

Tagging.

arianek’s picture

arianek commented

this seems like it would fit nicely in here: http://drupal.org/coding-standards if you can add it as a page... (and we'll have to get it reviewed)

figaro’s picture

figaro commented

Authenticated users do not have editing rights for creating a page on that section of the Drupal documentation / handbook.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

mmm, forgot about this issue... could we move it into the Drupal core » javascript/markup component queue to gain some more visibility?

arianek’s picture

arianek commented
Project: Documentation » Drupal core
Version: » 7.x-dev
Component: New documentation » javascript
Issue tags: +jQuery, +theming

moving as requested.

mattyoung’s picture

mattyoung commented

subscribe

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

On 4., I'd especially like to see a rule for how to deal with .end() statements.

Based on quite advanced jQuery code in Dreditor, I came up with the following:

    // Unwrap basic issue data.
    $form
      .find('fieldset:first')
        .removeClass('collapsible').addClass('fieldset-flat')
        .find('.fieldset-wrapper')
          // Hide note about issue title for n00bs.
          .find('.description:first').hide().end()
          .end()
        .end()
      .find('label[for="edit-comment"]').hide();

Note that this example code doesn't really exist in Dreditor, but I definitely ran into the coding style issue of having multiple .find() and multiple .end() statements acting on different sub-selectors.

Essentially, I put the .end() on the same indentation level as the inner code of a .find() sub-selector, so it is clear(er) that the .end() ends the last .find() and not the previous/parent .find() on the same indentation level.

--

Additionally, what about jQuery methods that take multiple functions as arguments? E.g., also copying from Dreditor:

    // Add hide/show button to temporarily dismiss Dreditor.
    $('<input id="dreditor-hide" class="dreditor-button" type="button" value="Hide" />')
      .toggle(
        function () {
          self.hide();
        },
        function () {
          self.show();
        }
      )
      .appendTo($actions);
    // Add cancel button to tear down Dreditor.

Here, an alternative would be to write:

      .toggle(function () {
        self.hide();
      },
      function () {
        self.show();
      })
      .appendTo($actions);

...but that looks kinda confusing IMHO.

figaro’s picture

figaro commented

As noted in #4, can someone from the documentation team please open a page for this?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

@figaro: Would make sense to discuss and agree something before creating a page, no? :)

arianek’s picture

arianek commented

@figaro, @sun, i'm watching here, and happy to create a page as soon as there is the content is ready enough to be posted (if sun can't create it himself) - just let me know where you'd like it added.

joshuajabbour’s picture

joshuajabbour commented

Personally, I'd love for us to model our JS standards on jQuery's: http://docs.jquery.com/JQuery_Core_Style_Guidelines (obv the tabs vs. spaces might prob be a deviation).

The coding styles mentioned in #1 etc are good additions not explicitly declared by jQuery however...

ericduran’s picture

ericduran commented
Issue tags: +JavaScript

We also have this relevant issue #1337022: [policy, no patch] Create/adopt JavaScript docs standards compatible with a JS documentation parser.

Also tagging with JavaScript. It would make sense if we use the JavaScript tag for all JS related issue on drupal. ;)

robloach’s picture

robloach
he/him
commented
Status: Active » Closed (duplicate)

Let's just set this one as a duplicate ;-) .

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Closed (duplicate) » Active

Excuse me? This is about code, not documentation.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 7.x-dev » 8.x-dev

As a note, we do have a page for JavaScript coding standards:
http://drupal.org/node/172169

Which items being discussed here need to be added there? I don't think we need/want to have standards specific to JQuery -- the standards we make should apply to all JavaScript, right?

bowersox’s picture

bowersox commented

I think adding a these rules to our own Javascript coding standards is best. We can probably combine rules 2-4 into one guideline. We can probably also combine rules 1 and 7 into the same guideline.

@sun, can you suggest guidelines to address the questions you posed in comment 8 above?

@jhodgdon, some of these guidelines are particular to jQuery, so I would suggest a new section about jQuery at the bottom of the Javascript Coding Standards document.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

My point is that the rules are really not JQuery-specific. They are specific to constructs in JavaScript where a function is defined on the fly as part of a function call, which JQuery does all over the place, but in theory, they could be used in other JavaScript, not just JQuery. For example:

var x = foo(a, b, function() { whatever; });

It doesn't have to be $() to have a function being defined inside a function call.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes

swapped out pre tags for code tags and fixed indenting

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Project: Drupal core » Drupal Technical Working Group
Version: 8.0.x-dev »
Component: javascript » Documentation

Coding standards decisions are now supposed to be made by the TWG

drumm’s picture

drumm
he/him
Location NY, US
commented
Issue summary: View changes
tizzo’s picture

tizzo commented
Project: Drupal Technical Working Group » Coding Standards

Moving this issue to the Coding Standards queue per the new workflow defined in #2428153: Create and document a process for updating coding standards.

droplet’s picture

droplet commented
Parent issue: » #1778828: [policy, no patch] Update JS coding standards
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: -JavaScript, -theming, -Coding standards
quietone’s picture

quietone commented
Status: Active » Postponed (maintainer needs more info)

This issue began in 2009 and in Aug 2012 the "jQuery coding standards" page was added with frequent updates in 2013. Updates continued, at a slower pace until 2020. That alone makes we wonder if this issue is still relevant. Plus there has been not discussion here on the proposal since 2012.

So, is there work to do here? If there is add a comment, otherwise this issue will be closed after three months.

Thanks.

quietone’s picture

quietone commented

Also, considering that core is working to remove JQuery, #3052002: [meta] Replace JQuery with vanilla Javascript in core. I think we should not put effort into this.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I agree, I think we can close this issue.