[D8] Sticky Toolbar [#2973329]

A detailed description of what this project does:

This module allows each individual user to toggle whether the core toolbar is sticky (css position:fixed) or positioned relative to the rest of the page (css position:absolute).

A link to project page.

https://www.drupal.org/project/sticky_toolbar

A git clone command

git clone --branch 8.x-1.x https://git.drupal.org/project/sticky_toolbar.git

A list of links to reviews of other project applications that bemarlan did.

N/a

Comments

Comment #1

16 May 2018 at 14:32

bemarlan created an issue. See original summary.

Comment #2

sleitner commented 30 June 2018 at 21:19

Status:

Needs review

» Needs work

Automated Review

There are some errors reported by automated review tools, did you already check them? See https://pareview.sh/pareview/https-git.drupal.org-project-sticky_toolbar... :

Git errors:

There is still a master branch, make sure to set the correct default branch: https://www.drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in https://www.drupal.org/node/1127732
The following git branches do not match the release branch pattern, you should remove/rename them. See https://www.drupal.org/node/1015226

Review of the 8.x-1.x branch (commit 7705499):

The sticky_toolbar.module does not implement hook_help().
Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
ESLint has found some issues with your code (please check the JavaScript coding standards).
DrupalPractice has found some issues with your code, but could be false positives.
No automated test cases were found, did you consider writing PHPUnit tests? This is not a requirement but encouraged for professional software development.

Manual Review

Individual user account: Yes
No duplication: Yes
Master Branch: No, see pareview.sh tests
Licensing: Yes
3rd party assets/code: Yes
README.txt/README.md: No: fix https://www.drupal.org/project/sticky_toolbar/issues/2981572
Code long/complex enough for review: Yes
Secure code: Yes
Coding style & Drupal API usage: No, see pareview.sh tests

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Comment #3

bemarlan

English

Ohio, USA

commented 9 August 2018 at 14:48

Issue summary:	View changes
Status:	Needs work	» Needs review

Comment #4

sleitner commented 9 August 2018 at 15:04

Status:

Needs review

» Needs work

Beverly, you deleted the closing curly bracket of the class in StickySettingController.php . Correct this first and then recheck your git repository at https://pareview.sh/ for further errors .

Pareview update:

Git errors:

The following git branches do not match the release branch pattern, you should remove/rename them. See https://www.drupal.org/node/1015226
```
  remotes/origin/8.x-1.0
  remotes/origin/8.x-1.x-dev
```

Review of the 8.x-1.x branch (commit e9dbca2):

PHP Parse error: syntax error, unexpected end of file, expecting function (T_FUNCTION) or const (T_CONST) in ./src/Controller/StickySettingController.php on line 86
Errors parsing ./src/Controller/StickySettingController.php
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

ESLint has found some issues with your code (please check the JavaScript coding standards).

/var/vhosts/c214000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/js/sticky-toolbar-reset.js: line 5, col 1, Error - Trailing spaces not allowed. (no-trailing-spaces)

1 problem

DrupalPractice has found some issues with your code, but could be false positives.


FILE: .../pareviewsh/pareview_temp/src/Controller/StickySettingController.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------
 36 | WARNING | Variable $this is undefined.
 50 | WARNING | Variable $this is undefined.
 60 | WARNING | Variable $this is undefined.
 79 | WARNING | Variable $this is undefined.
--------------------------------------------------------------------------


FILE: ...web/vendor/drupal/pareviewsh/pareview_temp/src/Form/SettingsForm.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 43 | WARNING | t() calls should be avoided in classes, use dependency
    |         | injection and $this->t() instead
--------------------------------------------------------------------------

Time: 193ms; Memory: 4Mb

No automated test cases were found, did you consider writing PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script.


FILE: ...00000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.txt
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 62 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...vendor/drupal/pareviewsh/pareview_temp/sticky_toolbar.links.menu.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 7 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: .../pareviewsh/pareview_temp/src/Controller/StickySettingController.php
--------------------------------------------------------------------------
FOUND 53 ERRORS AND 2 WARNINGS AFFECTING 48 LINES
--------------------------------------------------------------------------
  3 | ERROR   | [x] Namespaced classes, interfaces and traits should not
    |         |     begin with a file doc comment
  5 | ERROR   | [x] Doc comment short description must end with a full
    |         |     stop
 11 | WARNING | [x] Unused use statement
 16 | WARNING | [ ] Possible parse error: class missing opening or
    |         |     closing brace
 17 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 19 | ERROR   | [x] Whitespace found at end of line
 20 | ERROR   | [x] Expected "int" but found "integer" for function
    |         |     return type
 23 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 23 | ERROR   | [ ] Invalid function name, expected get_user_id but found
    |         |     getUserId
 23 | ERROR   | [x] Expected 1 blank line before function; 0 found
 24 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 25 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 26 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 28 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 30 | ERROR   | [x] Whitespace found at end of line
 31 | ERROR   | [x] Expected "int" but found "integer" for function
    |         |     return type
 34 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 34 | ERROR   | [ ] Invalid function name, expected
    |         |     get_sticky_setting_data but found
    |         |     getStickySettingData
 35 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 36 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 38 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 39 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 41 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 43 | ERROR   | [x] Whitespace found at end of line
 44 | ERROR   | [x] Expected "int" but found "integer" for parameter type
 47 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 47 | ERROR   | [ ] Invalid function name, expected
    |         |     set_sticky_setting_data but found
    |         |     setStickySettingData
 48 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 50 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 51 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 53 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 55 | ERROR   | [x] Whitespace found at end of line
 56 | ERROR   | [x] Expected "int" but found "integer" for function
    |         |     return type
 59 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 59 | ERROR   | [ ] Invalid function name, expected get_setting but found
    |         |     getSetting
 60 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 61 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 63 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 63 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected
    |         |     "NULL" but found "null"
 64 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 6
 65 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 67 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 68 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 70 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 72 | ERROR   | [x] Whitespace found at end of line
 73 | ERROR   | [x] Expected "int" but found "integer" for parameter type
 75 | ERROR   | [x] Whitespace found at end of line
 78 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
 78 | ERROR   | [ ] Invalid function name, expected set_setting but found
    |         |     setSetting
 79 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 81 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 82 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 83 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 84 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 85 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 2
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 49 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...web/vendor/drupal/pareviewsh/pareview_temp/src/Form/SettingsForm.php
--------------------------------------------------------------------------
FOUND 17 ERRORS AND 2 WARNINGS AFFECTING 17 LINES
--------------------------------------------------------------------------
  3 | ERROR   | [x] Namespaced classes, interfaces and traits should not
    |         |     begin with a file doc comment
  5 | ERROR   | [x] Doc comment short description must end with a full
    |         |     stop
 11 | WARNING | [x] Unused use statement
 13 | WARNING | [x] Unused use statement
 14 | ERROR   | [x] There must be one blank line after the last USE
    |         |     statement; 2 found;
 24 | ERROR   | [ ] Public method name "SettingsForm::getFormID" is not
    |         |     in lowerCamel format
 24 | ERROR   | [x] Expected 1 blank line before function; 0 found
 29 | ERROR   | [ ] Doc comment short description must start with a
    |         |     capital letter
 29 | ERROR   | [x] Doc comment short description must end with a full
    |         |     stop
 35 | ERROR   | [x] Short array syntax must be used to define arrays
 40 | ERROR   | [x] Short array syntax must be used to define arrays
 41 | ERROR   | [x] Array indentation error, expected 6 spaces but found
    |         |     4
 42 | ERROR   | [x] Array indentation error, expected 6 spaces but found
    |         |     4
 43 | ERROR   | [x] Array indentation error, expected 6 spaces but found
    |         |     4
 44 | ERROR   | [x] Array closing indentation error, expected 4 spaces
    |         |     but found 5
 51 | ERROR   | [x] Whitespace found at end of line
 58 | ERROR   | [ ] is_integer() is a function name alias, use is_int()
    |         |     instead
 65 | ERROR   | [x] Expected 1 blank line after function; 0 found
 66 | ERROR   | [x] The closing brace for the class must have an empty
    |         |     line before it
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 16 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...endor/drupal/pareviewsh/pareview_temp/sticky_toolbar.permissions.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 5 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...eb/vendor/drupal/pareviewsh/pareview_temp/sticky_toolbar.routing.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 8 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...1/web/vendor/drupal/pareviewsh/pareview_temp/sticky_toolbar.info.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------
  1 | WARNING | [ ] Remove "version" from the info file, it will be added
    |         |     by drupal.org packaging automatically
 17 | ERROR   | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Time: 543ms; Memory: 4Mb

Comment #5

sleitner commented 9 August 2018 at 15:03

Issue summary:

View changes

Comment #6

bemarlan

English

Ohio, USA

commented 9 August 2018 at 15:23

Status:

Needs work

» Needs review

Class closing error was updated.

Comment #7

sleitner commented 9 August 2018 at 15:36

Status:

Needs review

» Needs work

Automated Review

Git errors:

The following git branches do not match the release branch pattern, you should remove/rename them. See https://www.drupal.org/node/1015226
```
  remotes/origin/8.x-1.0
  remotes/origin/8.x-1.x-dev
```

Review of the 8.x-1.x branch (commit 673cf8e):

Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

ESLint has found some issues with your code (please check the JavaScript coding standards).

/var/vhosts/c214000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/js/sticky-toolbar-reset.js: line 5, col 1, Error - Trailing spaces not allowed. (no-trailing-spaces)

1 problem

DrupalPractice has found some issues with your code, but could be false positives.


FILE: .../pareviewsh/pareview_temp/src/Controller/StickySettingController.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
--------------------------------------------------------------------------
 24 | WARNING | \Drupal calls should be avoided in classes, use
    |         | dependency injection instead
 35 | WARNING | \Drupal calls should be avoided in classes, use
    |         | dependency injection instead
 48 | WARNING | \Drupal calls should be avoided in classes, use
    |         | dependency injection instead
 82 | WARNING | \Drupal calls should be avoided in classes, use
    |         | dependency injection instead
 83 | WARNING | \Drupal calls should be avoided in classes, use
    |         | dependency injection instead
--------------------------------------------------------------------------


FILE: ...web/vendor/drupal/pareviewsh/pareview_temp/src/Form/SettingsForm.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 43 | WARNING | t() calls should be avoided in classes, use dependency
    |         | injection and $this->t() instead
--------------------------------------------------------------------------

Time: 183ms; Memory: 4Mb

No automated test cases were found, did you consider writing PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script.


FILE: ...00000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.txt
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 62 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...vendor/drupal/pareviewsh/pareview_temp/sticky_toolbar.links.menu.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 7 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: .../pareviewsh/pareview_temp/src/Controller/StickySettingController.php
--------------------------------------------------------------------------
FOUND 17 ERRORS AND 1 WARNING AFFECTING 18 LINES
--------------------------------------------------------------------------
  3 | ERROR   | [x] Namespaced classes, interfaces and traits should not
    |         |     begin with a file doc comment
  5 | ERROR   | [x] Doc comment short description must end with a full
    |         |     stop
 11 | WARNING | [x] Unused use statement
 19 | ERROR   | [x] Whitespace found at end of line
 20 | ERROR   | [x] Expected "int" but found "integer" for function
    |         |     return type
 23 | ERROR   | [x] Expected 1 blank line before function; 0 found
 30 | ERROR   | [x] Whitespace found at end of line
 31 | ERROR   | [x] Expected "int" but found "integer" for function
    |         |     return type
 43 | ERROR   | [x] Whitespace found at end of line
 44 | ERROR   | [x] Expected "int" but found "integer" for parameter type
 55 | ERROR   | [x] Whitespace found at end of line
 56 | ERROR   | [x] Expected "int" but found "integer" for function
    |         |     return type
 63 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected
    |         |     "NULL" but found "null"
 72 | ERROR   | [x] Whitespace found at end of line
 73 | ERROR   | [x] Expected "int" but found "integer" for parameter type
 75 | ERROR   | [x] Whitespace found at end of line
 85 | ERROR   | [x] Expected 1 blank line after function; 0 found
 86 | ERROR   | [x] The closing brace for the class must have an empty
    |         |     line before it
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 18 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...web/vendor/drupal/pareviewsh/pareview_temp/src/Form/SettingsForm.php
--------------------------------------------------------------------------
FOUND 17 ERRORS AND 2 WARNINGS AFFECTING 17 LINES
--------------------------------------------------------------------------
  3 | ERROR   | [x] Namespaced classes, interfaces and traits should not
    |         |     begin with a file doc comment
  5 | ERROR   | [x] Doc comment short description must end with a full
    |         |     stop
 11 | WARNING | [x] Unused use statement
 13 | WARNING | [x] Unused use statement
 14 | ERROR   | [x] There must be one blank line after the last USE
    |         |     statement; 2 found;
 24 | ERROR   | [ ] Public method name "SettingsForm::getFormID" is not
    |         |     in lowerCamel format
 24 | ERROR   | [x] Expected 1 blank line before function; 0 found
 29 | ERROR   | [ ] Doc comment short description must start with a
    |         |     capital letter
 29 | ERROR   | [x] Doc comment short description must end with a full
    |         |     stop
 35 | ERROR   | [x] Short array syntax must be used to define arrays
 40 | ERROR   | [x] Short array syntax must be used to define arrays
 41 | ERROR   | [x] Array indentation error, expected 6 spaces but found
    |         |     4
 42 | ERROR   | [x] Array indentation error, expected 6 spaces but found
    |         |     4
 43 | ERROR   | [x] Array indentation error, expected 6 spaces but found
    |         |     4
 44 | ERROR   | [x] Array closing indentation error, expected 4 spaces
    |         |     but found 5
 51 | ERROR   | [x] Whitespace found at end of line
 58 | ERROR   | [ ] is_integer() is a function name alias, use is_int()
    |         |     instead
 65 | ERROR   | [x] Expected 1 blank line after function; 0 found
 66 | ERROR   | [x] The closing brace for the class must have an empty
    |         |     line before it
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 16 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...endor/drupal/pareviewsh/pareview_temp/sticky_toolbar.permissions.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 5 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...eb/vendor/drupal/pareviewsh/pareview_temp/sticky_toolbar.routing.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 8 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...1/web/vendor/drupal/pareviewsh/pareview_temp/sticky_toolbar.info.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------
  1 | WARNING | [ ] Remove "version" from the info file, it will be added
    |         |     by drupal.org packaging automatically
 17 | ERROR   | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Time: 497ms; Memory: 4Mb

Comment #8

bemarlan

English

Ohio, USA

commented 23 August 2018 at 15:48

Status:

Needs work

» Needs review

Branch has been updated and is now passing automated review: https://pareview.sh/pareview/https-git.drupal.org-project-sticky_toolbar...

Comment #9

sleitner commented 23 August 2018 at 16:14

Status:

Needs review

» Reviewed & tested by the community

Comment #10

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 23 August 2018 at 17:57

Assigned:	Unassigned	» avpaderno
Status:	Reviewed & tested by the community	» Fixed

Thank you for your contribution!

I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-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 dedicated reviewers as well.

Comment #11

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 23 August 2018 at 17:57

Comment #12

6 September 2018 at 17:59

Status:

Fixed

» Closed (fixed)

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

[D8] Sticky Toolbar

Comments

Comment #1

Comment #2

Automated Review

Manual Review

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Automated Review

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

News items

Our community

Documentation

Drupal code base

Governance of community