Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[DO NOT MERGE] Using TextViewSizeAwareTouchListener with Text views (scaling refactor part 4) #210

Closed
wants to merge 4 commits into from
Closed

[DO NOT MERGE] Using TextViewSizeAwareTouchListener with Text views (scaling refactor part 4) #210

wants to merge 4 commits into from

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Nov 25, 2019 •
edited
Loading

Builds on top of #209.

Adding the new listener to be used with actual TextViews (already used for emoji at this point).

In this type of view we also need to detect taps (through GestureDetector) so we can open the text editor dialog and fire up the keyboard for editing. 361f418

To test:

  1. open the app and capture an image
  2. tap on the Tt control to add text
  3. insert some text
  4. observe the text gets inserted and you can drag it around and resize (zoom in/out).
  5. also try and insert some emji within the text, and observe it gets handled correctly.

Bonus test (to make sure other functionality is compatible):

  1. somewhere in the code, call testSticker() in ComposeLoopFrameActivity, for example:
 text_add_button_group.setOnClickListener {
             addNewText()
           testSticker()
       }
  1. run it
  2. capture an image
  3. in the composer screen, tap on the Tt button
  4. observe the editor dialog is shown, dismiss it
  5. observe an image with an animated gif has been added
  6. move the image around, resize it, rotate it.

@peril-automattic
Copy link

peril-automattic bot commented Nov 25, 2019 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

@mzorz mzorz changed the title WIP - Using TextViewSizeAwareTouchListener with Text views Using TextViewSizeAwareTouchListener with Text views Nov 25, 2019
mzorz
mzorz commented Nov 25, 2019
View reviewed changes
photoeditor/src/main/java/com/automattic/photoeditor/PhotoEditor.kt
@@ -413,11 +415,20 @@ class PhotoEditor private constructor(builder: Builder) :
when (viewType) {
ViewType.TEXT -> {
rootView = layoutInflater.inflate(R.layout.view_photo_editor_text, null)
if (rootView.tvPhotoEditorText != null && mDefaultTextTypeface != null) {
rootView.tvPhotoEditorText.gravity = Gravity.CENTER
val txtTextView = rootView.tvPhotoEditorText
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this section in the when () clause is basically copied from the EMOJI case except for the mDefaultTextTypeface which is different from mDefaultEmojiTypeface. Can possibly try and refactor further.

@mzorz mzorz requested a review from aforcier November 25, 2019 19:56
mzorz
mzorz commented Nov 25, 2019
View reviewed changes
photoeditor/src/main/java/com/automattic/photoeditor/PhotoEditor.kt
@@ -381,7 +383,7 @@ class PhotoEditor private constructor(builder: Builder) :
* @param rootView rootview of image,text and emoji
*/
private fun addViewToParent(rootView: View, viewType: ViewType, sourceUri: Uri? = null) {
if (viewType != EMOJI) {
if (viewType != EMOJI && viewType != TEXT) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both EMOJI and TEXT view types use the same AutoResizeTextView class instead of plain old TextView so, we need to set a width/height to start with (instead of wrap_content), which will later on be adjusted on fontSize accordingly.

mzorz
mzorz commented Nov 25, 2019
View reviewed changes
photoeditor/src/main/java/com/automattic/photoeditor/PhotoEditor.kt
}
txtTextView.gravity = Gravity.CENTER
// txtTextView.setLayerType(View.LAYER_TYPE_SOFTWARE, null)
val maxHeight = parentView.height
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to change as we get #206 merged, and do the same here.

Copy link
Contributor Author

@mzorz mzorz Nov 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to change as we get #206 merged, and do the same here.

Handled in #211

mzorz
mzorz commented Nov 25, 2019
View reviewed changes
photoeditor/src/main/java/com/automattic/photoeditor/gesture/TextViewSizeAwareTouchListener.kt
this.onGestureControl = onGestureControl
}

private inner class GestureListener : GestureDetector.SimpleOnGestureListener() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SimpleOnGestureListener gives us the ability to listen to clicks in a distinguishable way mostly. Could simplify and listen to MOVE_UP events given we're already processing them but, prefer to avoid false positives and rely on the platform mechanism for onClick().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used so we can show the Editor dialog when the user taps on an existing view of type TEXT.

@mzorz
Copy link
Contributor Author

mzorz commented Nov 25, 2019

This one should be ready for review now @aforcier 🙇

@mzorz mzorz mentioned this pull request Nov 25, 2019
Closed
7 tasks
This was referenced Nov 26, 2019
Closed
Merged
@mzorz mzorz changed the title Using TextViewSizeAwareTouchListener with Text views Using TextViewSizeAwareTouchListener with Text views (scaling refactor part 4) Nov 29, 2019
@mzorz mzorz changed the title Using TextViewSizeAwareTouchListener with Text views (scaling refactor part 4) [DO NOT MERGE] Using TextViewSizeAwareTouchListener with Text views (scaling refactor part 4) Dec 2, 2019
@mzorz mzorz mentioned this pull request Dec 2, 2019
Closed
@mzorz mzorz closed this Dec 31, 2019
@mzorz mzorz deleted the issue/207-fix-text-handling branch July 29, 2020 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aforcier aforcier Awaiting requested review from aforcier

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@mzorz