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.

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

Bugfix: fix WindowType of Tooltip #30

Merged
merged 4 commits into from
Jan 6, 2022
Merged

Bugfix: fix WindowType of Tooltip #30

merged 4 commits into from
Jan 6, 2022

Conversation

CoXier
Copy link
Contributor

@CoXier CoXier commented Jan 5, 2022

Because of i18n, the title may be not "Tooltip" in some country. For example, "提示" in Chinese.

So I use string id to get string.

Because of i18n, the title may be not "Tooltip" in some country. For example, "提示" in Chinese.
@drakeet
Copy link

drakeet commented Jan 5, 2022

(FYI)

There's a compat version of TooltipPopup, how about also supporting it?

eg.

+ title == androidx.appcompat.widget.TooltipPopup.getClass().getSimpleName(),
title == tooltipString -> TOOLTIP

I noticed that this library does not rely on the appcompat, so the above may not be possible, so we can also consider providing a View.windowTitle: String? extension, so that users can determine more types by themselves.

image

@CoXier
Copy link
Contributor Author

CoXier commented Jan 5, 2022

@drakeet
Actually curtains only check com.android.internal.view.TooltipPopup.

UI test:

  @Test fun tooltip_view_has_TOOLTIP_type() {
    assumeSdkAtLeast(26, "View.setTooltipText() was added in API 26")
    onActivity { activity ->
      val contentView = TextView(activity).apply { text = "Yo" }
      contentView.tooltipText = "Tooltip"
      activity.setContentView(contentView)

      Curtains.onRootViewsChangedListeners.addUntilClosed(OnRootViewAddedListener { rootView ->
        assertThat(rootView.windowType).isEqualTo(TOOLTIP)
      }).use {
        contentView.performLongClick()
      }
    }
  }

As you see, when contentView.performLongClick(), view will show com.android.internal.view.TooltipPopup.

Android 12 Code: https://cs.android.com/android/platform/superproject/+/android-12.0.0_r4:frameworks/base/core/java/android/view/View.java;l=4769

So this pull request does solve the problem.

LOL, I guess that the author doesn't plan to handle androidx.appcompat.widget.TooltipPopup.

@pyricau
Copy link
Member

pyricau commented Jan 6, 2022

Thanks for the fix!

I filed #31 for the AndroidX tooltip. Contributions welcome!

@pyricau pyricau merged commit eff3309 into square:main Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants