cc: restructure edit page JS for improved maintainability #1646
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the JS script for the custom command edit page to be more maintainable and extensible.
As noted previously in #1619 (comment), the current JS script for the custom command edit page has grown rather complex and difficult to understand over time. Particularly egregious is the current logic for showing/hiding various parts of the page depending on the trigger type:
yagpdb/customcommands/assets/customcommands-editcmd.html
Lines 536 to 613 in 447a527
Though avoiding overabstraction is important, the sheer repetitiveness and density of this code (~80 lines with very minor differences between if branches, obscuring the function of the code) renders it extremely difficult to understand and maintain. The rest of the script is not much better in this regard.
This PR therefore attempts to refactor the entire script while not changing functionality. The best solution would be to use a frontend framework that allows for proper state management, à la React, but such a broad change is unfeasible at the moment.
The changes introduced are best reviewed by opening the new file separately as opposed to examining the diff.
General outline of changes
Broadly speaking, the responsibilities of the current JS script are as follows:
Require roles
menu for a command-type trigger, but not for an interval-type trigger.In this refactor, we concern ourselves mainly with improving the implementation of 2) and 3), which were previously rather ad-hoc (see the quoted segment of code above for 2), for instance.)
Though it may seem as though we have introduced some abstraction in this process, the end result is not only more modular but also drastically shorter than the previously implementation by about 40 lines overall.
To ensure minimal behavior change, I have visually compared the result on my selfhost with the corresponding page on YAGPDB and have confirmed, to the best of my ability, that there are no problematic differences. But this claim should of course be verified and tested more thoroughly before release.