Add tap-to-place text positioning - #592
Open
boardtc wants to merge 3 commits into
Open
Conversation
Author
|
Additional device-validation note: This change was tested successfully on a Pixel 9a after installing the rebuilt debug APK. Confirmed on-device:
|
Owner
|
@boardtc Please fix the tests first. |
Author
|
Addressed this. I pushed commit What changed:
|
There was a problem hiding this comment.
Pull request overview
This PR implements issue #589 by introducing explicit initial placement for text overlays (instead of always centering), adding a Position coordinate type and new PhotoEditor.addText(..., position) overloads, and updating the sample app UX to “tap-to-place” before entering text.
Changes:
- Added
Positionand plumbing throughPhotoEditor/PhotoEditorImpl/GraphicManagerto support initial text placement while preserving default centered behavior. - Updated sample app UI + instrumentation test flow to require a tap location before showing the text dialog.
- Updated documentation/release artifacts (README, CHANGELOG, version bump, contributing checks) and added unit coverage around centered vs positioned insertion.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents new Position API usage and updated sample app user flow. |
| photoeditor/src/test/java/ja/burhanrashid52/photoeditor/GraphicManagerTest.kt | Adds unit tests for default centered placement and explicit positioned placement. |
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/Position.kt | Introduces a pixel-based coordinate type for initial overlay placement. |
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/PhotoEditorImpl.kt | Implements the new addText overloads and routes optional Position into editor insertion. |
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/PhotoEditor.kt | Adds new addText(..., position) overloads to the public API. |
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/GraphicManager.kt | Applies centered vs positioned layout params when adding a new overlay view. |
| photoeditor/build.gradle | Bumps publish version to 3.1.1. |
| CONTRIBUTING.md | Adds guidance to run ./gradlew check and ./gradlew build before opening PRs. |
| CHANGELOG.md | Adds 3.1.1 release notes describing the feature, tests, and docs. |
| app/src/main/res/values/strings.xml | Adds tap-to-place guidance string. |
| app/src/main/res/values-pl/strings.xml | Adds Polish translation for tap-to-place guidance. |
| app/src/main/res/values-fr/strings.xml | Adds French translation for tap-to-place guidance. |
| app/src/main/res/layout/activity_edit_image.xml | Adds a transparent overlay view to capture tap-to-place input. |
| app/src/main/java/com/burhanrashid52/photoediting/EditImageActivity.kt | Implements tap-to-place workflow and routes tap coordinates into addText(..., position). |
| app/src/androidTest/java/com/burhanrashid52/photoediting/EditImageActivityTest.kt | Updates Espresso flows to include a placement tap before typing text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (position == null) { | ||
| params.addRule(RelativeLayout.CENTER_IN_PARENT, RelativeLayout.TRUE) | ||
| } else { | ||
| params.addRule(RelativeLayout.ALIGN_PARENT_START, RelativeLayout.TRUE) |
Comment on lines
+33
to
+36
| android:background="@android:color/transparent" | ||
| android:clickable="true" | ||
| android:focusable="true" | ||
| android:visibility="gone" |
Comment on lines
+162
to
+165
| If you want the text to start at a specific location instead of the default centered position, use the overload with a `Position`: | ||
|
|
||
| `mPhotoEditor.addText(inputText, colorCode, Position(80, 160));` | ||
|
|
Comment on lines
+39
to
+50
| /** | ||
| * This adds the text on the [PhotoEditorView] at the provided initial [position]. | ||
| * by default [TextView.setText] will be 18sp | ||
| * | ||
| * @param text text to display | ||
| * @param colorCodeTextView text color to be displayed | ||
| * @param position initial position in pixels from the top-left of the editor | ||
| */ | ||
| @SuppressLint("ClickableViewAccessibility") | ||
| fun addText(text: String, colorCodeTextView: Int, position: Position) { | ||
| addText(text, colorCodeTextView) | ||
| } |
Comment on lines
+63
to
+75
| /** | ||
| * This adds the text on the [PhotoEditorView] at the provided initial [position]. | ||
| * by default [TextView.setText] will be 18sp | ||
| * | ||
| * @param textTypeface typeface for custom font in the text | ||
| * @param text text to display | ||
| * @param colorCodeTextView text color to be displayed | ||
| * @param position initial position in pixels from the top-left of the editor | ||
| */ | ||
| @SuppressLint("ClickableViewAccessibility") | ||
| fun addText(textTypeface: Typeface?, text: String, colorCodeTextView: Int, position: Position) { | ||
| addText(textTypeface, text, colorCodeTextView) | ||
| } |
Comment on lines
+87
to
+98
| /** | ||
| * This adds the text on the [PhotoEditorView] at the provided initial [position]. | ||
| * by default [TextView.setText] will be 18sp | ||
| * | ||
| * @param text text to display | ||
| * @param styleBuilder text style builder with your style | ||
| * @param position initial position in pixels from the top-left of the editor | ||
| */ | ||
| @SuppressLint("ClickableViewAccessibility") | ||
| fun addText(text: String, styleBuilder: TextStyleBuilder?, position: Position) { | ||
| addText(text, styleBuilder) | ||
| } |
Owner
|
@boardtc Can you please check copilot code review comments ? |
Author
|
Addressed the Copilot review comments in commit Changes made:
Verification rerun under Java 21:
|
Author
|
Hi, just checking in on this PR now that the test fix and the Copilot review follow-ups have been addressed. Please let me know if there’s anything else you’d like changed and I’m happy to update it. |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR addresses issue #589 by adding support for initial text placement instead of always inserting text centered on the image.
What changed
Positiontype for overlay coordinatesPhotoEditor.addText(..., position)overloadsaddText(...)behavior when no position is suppliedTextREADME.mdfor:Tests / verification
Verified locally with:
Added/updated coverage includes:
GraphicManagerTestfor default centered insertion and explicit placementDevice verification:
Notes
v3.1.1after device verification