Skip to content

Modifier.sentryTag uses Modifier.Node#4029

Merged
romtsn merged 6 commits into
getsentry:7.x.xfrom
richkzad:sentrytag_modifier_node
Feb 6, 2025
Merged

Modifier.sentryTag uses Modifier.Node#4029
romtsn merged 6 commits into
getsentry:7.x.xfrom
richkzad:sentrytag_modifier_node

Conversation

@richkzad

@richkzad richkzad commented Jan 7, 2025

Copy link
Copy Markdown
Contributor

📜 Description

Converts Modifier.sentryTag to be implemented with Modifier.Node.

fixes #4028

💡 Motivation and Context

Currently, Modifier.sentryTag prevents composable skipping, due to creating a lambda within a non-@Composable function.

Assuming the @Composable annotation cannot be added (might be a breaking change if other developers have used Modifier.sentryTag outside of a @Composable context), then a Modifier.Node implementation will help avoid recomposition.

💚 How did you test it?

I did build a UI test for this, but was unsure how to run it.

package io.sentry.uitest.android

import androidx.compose.foundation.layout.Box
import androidx.compose.ui.Modifier
import androidx.compose.ui.test.SemanticsMatcher
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.sentry.compose.SentryModifier.sentryTag
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
class SentryModifierTest : BaseUiTest() {

    @get:Rule
    val rule = createComposeRule()

    @Test
    fun sentryModifierAppliesTag() {
        rule.setContent {
            Box(Modifier.sentryTag(testSentryTag))
        }
        rule.onNode(
            SemanticsMatcher(testSentryTag) {
                it.config.find { (key, _) -> key.name == "sentryTag" }?.value == testSentryTag
            }
        )
            .assertExists()
    }
}

private const val testSentryTag = "TestSentryTag"

📝 Checklist

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

@markushi

markushi commented Jan 8, 2025

Copy link
Copy Markdown
Member

Referencing richkzad#2 here as well, so we can get this ready for merging soon.

@richkzad

richkzad commented Jan 9, 2025

Copy link
Copy Markdown
Contributor Author

Thank you @markushi ! Merged in your changes.

@markushi
markushi changed the base branch from main to 7.x.x January 20, 2025 12:02
@markushi
markushi changed the base branch from 7.x.x to main January 27, 2025 11:46
@markushi
markushi requested a review from lcian as a code owner January 27, 2025 11:46
@markushi
markushi changed the base branch from main to 7.x.x January 27, 2025 11:47
@markushi
markushi force-pushed the sentrytag_modifier_node branch from b7c51b3 to f361763 Compare January 27, 2025 12:03
@romtsn
romtsn merged commit 47a3903 into getsentry:7.x.x Feb 6, 2025
romtsn added a commit that referenced this pull request Feb 7, 2025
* Modifier.sentryTag uses Modifier.Node

* Update Changelog

* Add UI test for SentryModifier

* Make sentrymodifier a robolectric test

* Remove redundant dep

---------

Co-authored-by: Markus Hintersteiner <[email protected]>
Co-authored-by: Roman Zavarnitsyn <[email protected]>
romtsn added a commit that referenced this pull request Feb 7, 2025
* Modifier.sentryTag uses Modifier.Node (#4029)

* Modifier.sentryTag uses Modifier.Node

* Update Changelog

* Add UI test for SentryModifier

* Make sentrymodifier a robolectric test

* Remove redundant dep

---------

Co-authored-by: Markus Hintersteiner <[email protected]>
Co-authored-by: Roman Zavarnitsyn <[email protected]>

* Fix compose test version

---------

Co-authored-by: Richard Z <[email protected]>
Co-authored-by: Markus Hintersteiner <[email protected]>
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.

Modifier.sentryTag forces recomposition.

3 participants