πŸ‡ΊπŸ‡ΈUnited States @RobbyMo

Account created on 6 March 2019, almost 6 years ago
  • Senior Solutions Architect at CTACΒ 
#

Recent comments

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for bringing this to our attention radheymkumar and for providing the patch sarwan_verma. This has been added to the 1.0.x branch.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Marking as RTBC.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Similar issue for me when going to the /admin/reports/config-inspector page after upgrading to the latest version:

Symfony\Component\Validator\Exception\UnexpectedTypeException: Expected argument of type "array", "null" given in Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraintValidator->validate() (line 23 of /var/www/html/web/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php).

After downgrading back to the previous version that I was using (2.1.0) the error goes away.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for updating Michael! This issue can be closed now.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for providing an update for this Vivek! One thing that I ask that you add into your MR is the changes for the other references to the old input textfield.

For example in aws_bedrock_chat.js:

var userMessage = $('input.aws-bedrock-chat-user-input', context).val();

In aws_bedrock_chat.css:

.aws-bedrock-chat-input .form-type-textfield
and
.aws-bedrock-chat-input input.aws-bedrock-chat-user-input:focus

In the ChatForm.php file when you change the 'user_input' #type to 'textarea' it now is resizable by the user. I would suggest adding the following to the 'user_input' form array to stop Drupal's default resizing capability on the the textarea:

'#resizable' => 'none',

Another thing that I noticed is that the textarea automatically expanded before it was necessary, I think it might be from function updateTextareaHeight() where you have textarea.style.height = ""; to reset the height. I think you might need to put in the explicit height there. Since you put 50px in the css, putting 50px here should reset it to the original height before determining if it needs to expand. This same reset will be needed after line 50 $('.aws-bedrock-chat-user-input', context).val(''); where the input is cleared after submission so that the textarea field returns to the default size after the message is sent.

The other changes look good, just let me know if you have any questions, thanks!

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Looks good, merged into 1.0.x. Thanks for contributing!

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thanks for bringing this to our attention, you are correct about the environment variable change from base_url to host. We have updated the project page to reflect this change. Also, the Host URL can be found from the steps in the "Retrieve Sharepoint List URL(s)" section on the readme file. We will keep this issue open until the readme is updated to make things clearer.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Verified Drupal 10 compatibility issues resolved from merge request 4 on 2.0.x branch testing on Drupal 10.1.8.

I think this is a great little popup module. After configuring the popin settings and placing the block the popup displayed and cookie was set as expected.

One thing to note since 'core: 8.x' is not included in the popin.info.yml file this will not work with Drupal core versions below 8.7.7 - see https://www.drupal.org/node/3070687 β†’ . If you would like for this to be usable on all Drupal 8 versions I would suggest adding the 'core: 8.x' to the file as well, otherwise updating the core_version_requirement to be ^8.8 || 9 || 10 will ensure that the module installs on the correct versions that recognize the core_version_requirement key.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Verified Drupal 10 compatibility issues resolved with patch from #2. Patch verified on Drupal 10.1.8.

After adding a form id to the settings page the form correctly provides a confirmation popup to notify that changes might not be saved. This is a nice little module, one suggestion that I have for the future would be to trim the comma separated form ID values from settings page as people often put spaces after commas.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for the feedback. This item has been removed in the 1.0.x-dev branch as it is no longer necessary as the webform module is now a dependency as you mentioned. I will be looking to make this optional again in the near future and will incorporate in your suggestions. For my education would you be able to elaborate on why this is considered a security issue?

Also do you know at what point the module is deemed RTBC? Is it time based (X days without someone finding an issue) or does someone specific needs to do a specific validation or is there different criteria? Just wondering since we're looking to add in additional functionality but per the submission guidelines until this gets approved I am the only one who can make code updates to the project. Thanks!

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for the feedback. These items have all been addressed in the 1.0.x-dev branch. I noticed phpcs flagged the constructor short descriptions for being longer than 80 characters due to the class namespaces but from https://www.drupal.org/docs/develop/standards/php/php-coding-standards β†’ it mentions "Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 characters." so I assume that it's fine. Please let me know if there are any issues.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for the feedback. These items have all been addressed in the 1.0.x-dev branch.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Thank you for the feedback. These items have all been updated in the 1.0.x-dev branch.

πŸ‡ΊπŸ‡ΈUnited States RobbyMo

Verified Drupal 10 compatibility issues resolved with patch from #2. Patch verified on Drupal 10.1.2.

Production build 0.71.5 2024