Closed briri closed 1 year ago
1 Error | |
---|---|
:no_entry_sign: | Please include a CHANGELOG entry.
|
1 Warning | |
---|---|
:warning: | There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior. \n Ignore this warning if the PR ONLY contains translation.io synced updates. |
Generated by :no_entry_sign: Danger
@briri . Lots of changes here. I hope the feature tests helped to validate that everything is still behaving. What do you need from me for a review?
They were helpful. Thanks again for adding them. I just wanted to let you see the changes before I merged them in case anything jumped out. If not, then I'll go ahead and merge and do some additional testing once its out on the server
I did not map the js changes feature by feature as I reviewed the file, but the changes I saw look good. I was excited to see that you had a way to retire the jQuery html construction that was in the preferences.js file and re-use existing approaches. It also looked like you were able to standardize the simple form validations that were in place.
@terrywbrady I switched the remove links in the modal form and the preferences pages to buttons for accessibility. Then I did some refactoring to consolidate some of the JS and to bring it closer to the way we've done things elsewhere in the DB. I also updated some CSS and HTML based on those changes