[Feature:Developer] Consolidate Manage Sections javascript#12633
Conversation
Copied style for icon from user-profile.css
- moved styling to server.css - fixed indentation of button - restored submit button
Also removed comment from server.css
Rkoester47
changed the title
automateprojectmangement
Bot
moved this from Work in Progress
to Seeking Reviewer
in Submitty Development
John-Roy123
left a comment
John-Roy123
left a comment
There was a problem hiding this comment.
Following the instructions in the PR, all behaviors are working as intended (Creating new section with and without course ID, deleting section, and editing course IDs for existing courses). I was unable to find any issues with your implementation. The code looks very readable and still works. Good stuff!
github-project-automation
Bot
moved this from Seeking Reviewer
to Awaiting Maintainer Review
in Submitty Development
aconfo
left a comment
aconfo
left a comment
There was a problem hiding this comment.
I performed a functionality review. I tried adding various section names, and course IDs. I couldn't find an obvious way to break the site. I tried comparing my testing with and without the changes and couldn't find any difference between the two. Besides that, I also tried interacting with manage rotating sections and again I couldn't find anything different with or without it. I also tried adding sections that already existed which it prevented. All and all it looks good and I couldn't find any problems.
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Work in Progress
in Submitty Development
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
JManion32
left a comment
JManion32
left a comment
There was a problem hiding this comment.
Functionality is unchanged, code looks great. This abstraction separates concerns, opening the door for future changes such as migrating JS files to TS, Twig to Vue, etc. Nice work with this PR, and page as a whole!
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
williamjallen
left a comment
williamjallen
left a comment
There was a problem hiding this comment.
@Rkoester47 One quick fix and then this should be good to go.
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Work in Progress
in Submitty Development
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
Rkoester47
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Done
in Submitty Development
7 participants
Why is this Change Important & Necessary?
Closes #12632
Currently, the RotatingSectionsForm.twig file, which governs the Manage Sections page, has accumulate a lot of inline javascript. After many new features and functionality tweaks, this has become too much.
What is the New Behavior?
Now, the Javascript functions that are used by the Manage Sections page exist in rotating-sections.js. They are safely accessed from here and all of the functionality of Manage Sections remains intact.
What steps should a reviewer take to reproduce or test the bug or new feature?
Sign in as instructor, navigate to Sample > Manage Sections, and test all of the functionality of the page.
Automated Testing & Documentation
Other information
This is not a breaking change