- Issue created by @joevagyok
- Status changed to Needs review
about 1 year ago 3:47am 25 August 2023 - last update
about 1 year ago 6 pass - 🇦🇷Argentina tguerineau
In order to address the issue I made the following changes:
- Refactored the JavaScript code to resolve ESLint errors, Used eslint for Drupal JS coding standards → .
- Simplified the initialization of the ml variable within the local script scope.Manual Testing:
- Checked the retention of the previous maxlength value set on a field, including summaries.
- Verified the saving of maxlength settings.
- Ensured that each maxlength limit method operates as expected.
- Confirmed that changing maxlength limit methods works correctly.
- Tested updating from the latest release to the newest version of the main branch and didn't encounter any issues.Observations:
-I observed the code pattern const ml = ml || {}; which gives an impression that ml might be a global variable. However, after thoroughly checking the module's directory and the broader codebase, I didn't find any external declarations for ml.
-I've refactored the code to initialize ml only within the local scope of our script. Manual tests seem fine, but I'd appreciate if reviewers can validate this change and ensure there's no hidden dependency on a global ml. - Assigned to hbrokmeier
- 🇺🇸United States cedewey Denver, CO
Hi Tom'as,
Thanks so much for working on this issue! I tested your patch manually and it all works as expected.
Assigning to Heather for a code review.