jeziellago / compose-markdown

Markdown Text for Android Jetpack Compose 📋.
MIT License
567 stars 47 forks source link

Fix Issue #85 #96

Closed HXavier21 closed 2 months ago

HXavier21 commented 3 months ago

Fix #85 by adding a CustomTextView, which will continue passing the event to other components when there is no link clicked. Thus the below code can work as well:

MarkdownText(
    markdown = """
           ---
            __Advertisement :)__

            - __[nodeca](https://nodeca.github.io/pica/demo/)__ - high quality and fast image
              resize in browser.
            - __[babelfish](https://github.com/nodeca/babelfish/)__ - developer friendly
              i18n with plurals support and easy syntax.

            You will like those projects!

            ---
        """.trimIndent(),
    modifier = Modifier.pointerInput(Unit) {
        detectTapGestures(onLongPress = {
            Toast
                .makeText(this@MainActivity, "On long press", Toast.LENGTH_SHORT)
                .show()
        })
    }
)
HXavier21 commented 2 months ago

Find a fatal error, used a full-width punctuation in MainActivity when edit code in Github. Sorry for that.😭

AleksandarIlic commented 2 months ago

@HXavier21 @jeziellago This changes broke text selection. It's impossible to select text now.

HXavier21 commented 2 months ago

@HXavier21 @jeziellago This changes broke text selection. It's impossible to select text now.

I'll check it right away😢

HXavier21 commented 2 months ago

@HXavier21 @jeziellago This changes broke text selection. It's impossible to select text now.

It seems that selecting text also needs to trigger a long press event, causing a conflict. A workable but not good modification is:

when (isTextSelectable) {
    true -> TextView(factoryContext)
    false -> CustomTextView(factoryContext)
}.apply {
    //...
}

Another possible approach is to add control of isTextSelectable in CustomTextView and adapt the MovementMethod of selecting text in OnTouchEvent. Are there any other methods?

HXavier21 commented 2 months ago

Since enabling text selection means disabling the long press gesture, it has been proven in practice that the bug can be simply fixed by the following changes:

class CustomTextView(context: Context) : AppCompatTextView(context) {
    private var isTextSelectable: Boolean = false
    override fun onTouchEvent(event: MotionEvent): Boolean {
        performClick()
        if (isTextSelectable) {
            return super.onTouchEvent(event)
        } else {
            //...
        }
    }

    //...

    override fun setTextIsSelectable(selectable: Boolean) {
        super.setTextIsSelectable(selectable)
        isTextSelectable = selectable
    }
}
jeziellago commented 2 months ago

@HXavier21 could open a PR fixing it? In case you can't, I'll revert this change until we have a new and well-tested solution that doesn't break the current impl.

HXavier21 commented 2 months ago

@HXavier21 could open a PR fixing it? In case you can't, I'll revert this change until we have a new and well-tested solution that doesn't break the current impl.

I'll open a PR right away, and I'd like to know if there are any bugs similar to this one🤔

HXavier21 commented 2 months ago

@HXavier21 could open a PR fixing it? In case you can't, I'll revert this change until we have a new and well-tested solution that doesn't break the current impl.

See #108