Conversation
sohomdatta1
left a comment
sohomdatta1
left a comment
There was a problem hiding this comment.
Haven't looked too deeply at this but noted a few changes, we shouldn't be adding the .gitreview file and changes to package-lock.json. Those are probably artifacts of some system cofig changes?
Change-Id: I6ab200967673d8fe60c913ada22ed8090a7fa253
shraddhaa09
force-pushed
the
T413556-rotate-ocr
branch
2 times, most recently
from
f0473dd to
268ecd8
Compare
|
Thanks for the review. I’ve removed the unintended |
|
https://github.com/wikimedia/wikimedia-ocr/actions/runs/20956740425/job/60223218062?pr=147 brings up test/lint failures, probably a good idea to fix those as well (to my memory the tests should pass) |
…ions - Fix PHPCS violations in engine and controller files - Fix JavaScript and template indentation - Add comprehensive rotation parameter tests - Update deprecated set-output command in CI workflow - All tests passing, 0 PHPCS errors Change-Id: I06d263147ebc8cc1065b15b1d5fc40cc455ea1c5
- Add missing @param int $rotate documentation in EngineBase::getResult() docblock - Reorder GoogleCloudVisionEngine use statements alphabetically - Fix declare statement formatting Change-Id: Ib9c2f72d0c706344d0bff5267abdb01d3a6d61fc
- Move '// NEW' comments to their own lines (lines 398, 411) - Move rotation comment to separate line (line 126) These changes comply with MediaWiki coding standards that require comments to start on new lines rather than inline. Change-Id: Id9460b92d3aed5a370ec908103005ddf69ff272e
|
Hello @sohomdatta1, |
|
I'm confused here. I tested this locally, but the rotation does not work locally on Google OCR. @shraddhaa09 Did you test the functionality locally before pushing this change? |
|
There are rotation buttons, yes, and I can superficially rotate the image, but the image being sent to Google OCR is not rotated (not only that, when I click the "transcribe area" the rotation is reset while the crops are carried over like expected) |
|
Also, please remove the extraneous "NEW" comments and don't upload changes to the github action. |
There was a problem hiding this comment.
@sohomdatta1
I looked through the Codex icon set for the rotation controls, but I couldn’t find dedicated icons for rotate left or rotate right.
Before updating this further, I wanted to confirm the preferred approach here. Would you recommend:
-reusing existing Codex icons (for example undo/redo)
-switching to text-based buttons for now
-handling rotation icons through a future Codex addition?
Please let me know which option you’d prefer and I’ll update the implementation accordingly.
There was a problem hiding this comment.
You could just use the existing codex icons, but I'd feel that getting the PR functional w.r.t the rotation is more of a priority here!
3 participants
Adds rotation buttons to the OCR interface allowing users to rotate scanned images before running OCR.
Changes:
Fixes: https://phabricator.wikimedia.org/T413556