[1.0.x] Animate Fields - AOS

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

To reviewers: Please read How to review security advisory coverage applications → , What to cover in an application review → , and Drupal.org security advisory coverage application workflow → .

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

1. Fix PHPCS issues.

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml animate_fields_aos/

FILE: animate_fields_aos/animate_fields_aos.module
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------------
  17 | WARNING | [ ] Line exceeds 80 characters; contains 83 characters
 169 | ERROR   | [x] Expected 1 space after "="; 2 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: animate_fields_aos/animate_fields_aos.options.yml
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 171 | ERROR | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: animate_fields_aos/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 15 | WARNING | Line exceeds 80 characters; contains 116 characters
 19 | WARNING | Line exceeds 80 characters; contains 89 characters
----------------------------------------------------------------------


FILE: animate_fields_aos/src/AttributesManager.php
--------------------------------------------------------------------------------
FOUND 16 ERRORS AND 3 WARNINGS AFFECTING 17 LINES
--------------------------------------------------------------------------------
  19 | ERROR   | [x] The extends keyword must be on the same line as the class name
  19 | ERROR   | [x] The implements keyword must be on the same line as the class name
  83 | WARNING | [x] '@todo: implement this method.' should match the format '@todo Fix problem X here.'
 100 | ERROR   | [x] Expected 1 blank line after function; 2 found
 106 | WARNING | [x] '@todo: implement this method.' should match the format '@todo Fix problem X here.'
 123 | ERROR   | [x] Expected 1 blank line after function; 2 found
 126 | ERROR   | [ ] Inline doc block comments are not allowed; use "/* Comment */" or "// Comment" instead
 131 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 132 | WARNING | [ ] Line exceeds 80 characters; contains 97 characters
 132 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
 133 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 134 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 135 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
 136 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 8
 137 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 8
 138 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
 139 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 140 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 142 | ERROR   | [x] Line indented incorrectly; expected 3 spaces, found 2
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: animate_fields_aos/src/AttributesManagerInterface.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
  6 | WARNING | Line exceeds 80 characters; contains 86 characters
 25 | WARNING | Line exceeds 80 characters; contains 91 characters
 25 | ERROR   | Doc comment short description must end with a full stop
 25 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
--------------------------------------------------------------------------------

2. 1.x-dev is a wrong branch name, as branch names end with the literal .x. That branch needs to be removed.

3. Remove the commented code.

FILE: src/AttributesManager.php

  /**
   * {@inheritdoc}
   */
  /*
  public function getTagOptions() {
    $options = [
      AttributesManagerInterface::NO_DATA_ANIMATION_VALUE => $this->t('None (No wrapping HTML)'),
    ];
    foreach ($this->getDefinitions() as $id => $definition) {
      $options[$definition['group']][$id] = $this->t('@label (@tag)', [
        '@label' => $definition['label'],
        '@tag' => $id,
      ]);
    }
    return $options;
  }
  */

I have reviewed the changes, and they look fine to me.

Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

Manual review

Individual user account
yes
No duplication
yes - Does not cause
Master Branch
yes
Licensing
Yes
3rd party assets/code
Yes
README.txt/README.md
No

• Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 → .
  • The INTRODUCTION section is missing.
  • The REQUIREMENTS section is missing.
  • The INSTALLATION section is missing.
  • The CONFIGURATION section is missing.

  Code long/complex enough for review
  Yes
  Secure code
  yes
  Coding style & Drupal API usage
  Resolved after comment 5.

Thanks @nielsaers and apaderno !
Readme.md best practices are fixed now.

      // Add data-aos attribute from setting:
      if (!empty($animate_fields_aos_config['animation'])) {
        $vars['attributes']['data-aos'] = $animate_fields_aos_config['animation'];
      }

      // Add data-aos-anchor-placement attribute from setting:
      if (!empty($animate_fields_aos_config['anchor_placements'])) {
        $vars['attributes']['data-aos-anchor-placement'] = $animate_fields_aos_config['anchor_placements'];
      }

      // Add data-aos-easing attribute from setting:
      if (!empty($animate_fields_aos_config['easing'])) {
        $vars['attributes']['data-aos-easing'] = $animate_fields_aos_config['easing'];
      }

      // Add data-aos-offset attribute from setting:
      if (!empty($animate_fields_aos_config['offset'])) {
        $vars['attributes']['data-aos-offset'] = $animate_fields_aos_config['offset'];
      }

      // Add data-aos-delay attribute from setting:
      if (!empty($animate_fields_aos_config['delay'])) {
        $vars['attributes']['data-aos-delay'] = $animate_fields_aos_config['delay'];
      }

      // Add data-aos-duration attribute from setting:
      if (!empty($animate_fields_aos_config['duration'])) {
        $vars['attributes']['data-aos-duration'] = $animate_fields_aos_config['duration'];
      }

      // Add data-aos-mirror attribute from setting:
      if (!empty($animate_fields_aos_config['mirror'])) {
        $vars['attributes']['data-aos-mirror'] = $animate_fields_aos_config['mirror'];
      }

Those comments are not necessary, since they just describe what is clear from the code.

      // @visit: https://www.drupal.org/project/aos
      $vars['#attached']['library'][] = 'aos/aos';

@visit is not used in Drupal modules. If there is information given in the project page that people who install the module should read, that should be given in the README file, not linked in code that probably few people will read.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries → ' post on Responsible maintainers
• Best practices for creating and maintaining projects →
• Maintaining a drupal.org project with Git →
• Commit messages - providing history and credit →
• Release naming conventions → .
• Helping maintainers in the issue queues →

You can find more contributors chatting on the Slack → #contribute channel. So, come hang out and stay involved → .
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review → . I encourage you to learn more about that process and join the group of reviewers.

I thank all the reviewers.

Automatically closed - issue fixed for 2 weeks with no activity.