- ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
- The following points are just a start and don't necessarily encompass all of the changes that may be necessary
- A specific point may just be an example and may apply in other places
- A review is about code that does not follow the coding standards, contains possible security issue, or does not correctly use the Drupal API
- The single review points are not ordered, not even by importance
src/Controller/AjaxCartUpdateController.php
Since that class does not use methods from the parent class, it does not need to use
ControllerBase
as parent class. Controllers do not need to have a parent class; as long as they implement\Drupal\Core\DependencyInjection\ContainerInjectionInterface
, they are fine.$response['cart_block_quantity'][] = $order_item->getQuantity() . ' ' . t('x'); $response['cart_block_title'][] = $order_item->getTitle();
The translatable string should use placeholders, which would avoid concatenating a translatable string with a string. It would also give to translator people more context.
The code should also use$this->t()
, since it is not in a static method.src/Form/AjaxCartUpdateSettingsForm.php
/** * The typed config manager. * * @var \Drupal\Core\Config\TypedConfigManagerInterface */ protected $typedConfigManager; /** * Constructs a new AjaxCartUpdateSettingsForm. * * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The config factory. * @param \Drupal\Core\Config\TypedConfigManagerInterface $typed_config_manager * The typed config manager. */ public function __construct(ConfigFactoryInterface $config_factory, TypedConfigManagerInterface $typed_config_manager) { parent::__construct($config_factory); $this->typedConfigManager = $typed_config_manager; }
$typed_config_manager
is passed to the parent class, which already has a property for that.ajax_cart_update.module
// Attach translations to drupalSettings. $translations = [ 'items' => new TranslatableMarkup('items', [], ['context' => 'Cart block item count']), 'x' => new TranslatableMarkup('x', [], ['context' => 'Cart block quantity separator']), ]; $attachments['#attached']['drupalSettings']['ajaxCartUpdate']['translations'] = $translations;
JavaScript code that needs translatable strings uses
Drupal.t()
.