Skip to content

Core objects #34

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Core objects #34

wants to merge 16 commits into from

Conversation

jrosgiralt
Copy link
Collaborator

No description provided.

@greggmarshall
Copy link
Collaborator

Jordi, there are a few automated checks to clean up first

gitpod@labdoo3-web:/var/www/html$ phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/custom

FILE: /var/www/html/web/modules/custom/lbd_default_content/lbd_default_content.module
-------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------
 7 | ERROR | [x] Whitespace found at end of line
-------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/lbd_blocks/src/Plugin/Block/BlockHubEdoovillage.php
---------------------------------------------------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AND 4 WARNINGS AFFECTING 10 LINES
---------------------------------------------------------------------------------------------------------------------------------------------
 19 | ERROR   | [x] Opening brace should be on the same line as the declaration
 23 | ERROR   | [x] Expected 1 blank line before function; 0 found
 24 | ERROR   | [x] Opening brace should be on the same line as the declaration
 28 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 28 | WARNING | [ ] Only string literals should be passed to t() where possible
 33 | WARNING | [ ] Only string literals should be passed to t() where possible
 41 | ERROR   | [x] Opening brace should be on the same line as the declaration
 49 | ERROR   | [x] Opening brace should be on the same line as the declaration
 50 | WARNING | [ ] Unused variable $config.
 59 | ERROR   | [x] Opening brace should be on the same line as the declaration
 61 | ERROR   | [x] Expected 1 blank line after function; 0 found
---------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------------------

Time: 174ms; Memory: 6MB

and

gitpod@labdoo3-web:/var/www/html$ phpstan --level=5 analyze web/modules/custom
Note: Using configuration file /var/www/html/phpstan.neon.
 3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ----------------------------------------------------- 
  Line   lbd_blocks/src/Plugin/Block/BlockHubEdoovillage.php  
 ------ ----------------------------------------------------- 
  26     Undefined variable: $code                            
 ------ ----------------------------------------------------- 


                                                                                                                        
 [ERROR] Found 1 error                                                                                                  
                     

@jrosgiralt
Copy link
Collaborator Author

Sorry i had run the checks in the immediately before commit, but must have missed doing it in the last commit. I was able to resolve all of the errors/warnings except for one warning:

gitpod@labdoo3-web:/var/www/html$ ./vendor/bin/phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/custom

FILE: /var/www/html/web/modules/custom/lbd_blocks/src/Plugin/Block/BlockHubEdoovillage.php
------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------
 34 | WARNING | Only string literals should be passed to t() where possible
------------------------------------------------------------------------------------------

Time: 154ms; Memory: 6MB

The offending line is this one:

    return [
      '#markup' => $this->t($code),
    ];

The following fixes the warning, however it does not render the html markup correctly:

    $replacements['@code'] = $code;
    return [
      '#markup' => $this->t("@code", $replacements),
    ];

@greggmarshall
Copy link
Collaborator

You shouldn't put variables in the t function. Take a look at https://www.drupal.org/docs/7/api/localization-api/dynamic-strings-with-placeholders. It would be best to translate the strings before wrapping them with HTML.

@jrosgiralt
Copy link
Collaborator Author

jrosgiralt commented Mar 19, 2023

Thanks Gregg. I know we should not put variables in the t function. The correct way should be the second codeblock I gave you, where I use the replacements array. But doing so prevents the html markup from properly rendering on the webpage.

@greggmarshall
Copy link
Collaborator

Did you try the other placeholders in that documentation???

And again, it is better to translate the string(s) then wrap them in HTML.

@jrosgiralt
Copy link
Collaborator Author

Try with exclamation point instead of the “@“
But the right thing to do it to put the ts up there in the code and let them be out of the html code.

- Removed content editor role
- Created Labdoo Story content type using paragraph module
- Created Gallery content type
- Added logic to Plugin/Block/BlockHubEdoovillage.php to check if user has the correct right to access/edit an edoovillage/hub.
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.

2 participants