-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEATURE] Midi player measure navigation #952
base: master
Are you sure you want to change the base?
Conversation
Verovio toolkit new version Properties adjusted View Images and Verovio side by side
Make score view compatible with multiview Add sync hover effect for facsimile and score
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did just a rough first review. Before continuing with a detailed code review, please fix those issues:
- Add
.DS_Store
files to.gitignore
and don't check those files in. - Check and fix all the Codacy issues.
- Resolve the merge conflicts with
master
branch. - Separate genuine third-party Javascript libraries from your own code and/or changed files. As a rule of thumb each third-party library should be put in their own directory (like OpenLayers, jQuery, Toastify, etc.) while changed/own files should be placed in the directory of the Controller which uses the file (like PageView, Search, etc.). The reason is that we exclude third-party libraries from our code quality scans because they usually don't comply with our coding guidelines.
# Conflicts: # Classes/Common/MetsDocument.php # Classes/Controller/AbstractController.php # Classes/Controller/PageViewController.php # Configuration/TypoScript/setup.typoscript # Resources/Private/Language/de.locallang.xlf # Resources/Private/Templates/Navigation/Main.html # Resources/Private/Templates/PageView/Main.html # Resources/Public/JavaScript/AudioPlayer/mediaplayer.js # Resources/Public/JavaScript/PageView/PageView.js # Resources/Public/JavaScript/PageView/ScoreControl.js # Resources/Public/JavaScript/PageView/ScoreUtility.js # Resources/Public/JavaScript/PageView/SyncControl.js
… the AbstractController
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
# Conflicts: # Classes/Common/AbstractDocument.php # Classes/Common/IiifManifest.php # Classes/Common/MetsDocument.php # Classes/Controller/AbstractController.php # Classes/Controller/PageViewController.php # Classes/Controller/ToolboxController.php # Resources/Private/Language/Labels.xml # Resources/Private/Templates/Navigation/Main.html # ext_conf_template.txt # ext_localconf.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
No description provided.