Skip to content

Migrate Display time-sensitive notifications snippets - #1079

Open
hamen wants to merge 8 commits into
android:mainfrom
hamen:hamen/time-sensitive-notifications
Open

hamen wants to merge 8 commits into
android:mainfrom
hamen:hamen/time-sensitive-notifications

Conversation

@hamen

@hamen hamen commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Code snippets are for:

List of modifications:

  • Time-sensitive page: Migrated 5 Kotlin code snippets to TimeSensitive.kt.
  • Time-sensitive page: Added baseline_auto_awesome_24.xml vector drawable resource to resolve notification icon reference.
  • Time-sensitive page: Preserved snippet lines with standard exclude tags for compilable state declaration.

Snippets not migrated:

  • Line 24 because it is an AndroidManifest.xml permission block.
  • Line 211 because it is a Java snippet cataloged for removal per Kotlin-first migration guidelines (D15).

@kkuan2011 kkuan2011 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once comments are addressed, you can add Aaron for review and remove from "draft" state, thanks!

@hamen
hamen marked this pull request as ready for review September 15, 2026 17:30
@hamen
hamen requested a review from a team as a code owner September 15, 2026 17:30
@hamen
hamen requested a review from simona-anomis September 15, 2026 17:30
@snippet-bot

snippet-bot Bot commented Sep 15, 2026

Copy link
Copy Markdown

Here is the summary of changes.

You are about to add 5 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@kkuan2011
kkuan2011 requested a review from alabiaga September 15, 2026 17:31

@kkuan2011 kkuan2011 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah actually looks like the build fails:

/home/runner/work/snippets/snippets/compose/snippets/src/main/java/com/example/compose/snippets/notifications/TimeSensitive.kt:120: Error: To call Service.startForeground(), the element of manifest file must have the foregroundServiceType attribute specified [ForegroundServiceType]
startForeground(notificationId, notification)
~~~~~~~~~~~~~~~

Explanation for issues of type "ForegroundServiceType":
For targetSdkVersion >= 34, to call Service.startForeground(), the
element in the manifest file must have the foregroundServiceType
attribute specified.

https://github.com/android/snippets/actions/runs/35001625484/job/104492132218?pr=1079

…heck

The suppression silenced the lint check but left code that throws on
API 34 and higher, where startForeground() needs a foregroundServiceType
on the <service> element.

Declare the service with shortService, which needs no runtime permission
request, and drop the suppression. The service class stops being private,
because the framework instantiates it.

The region tag content does not change, so the snippet on the page stays
the bare startForeground() call.
@hamen

hamen commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@kkuan2011 the ForegroundServiceType build error is fixed in d9e66b1. The service declares android:foregroundServiceType="shortService" in the manifest, which is what Aaron asked for in his review, so the lint check has nothing left to report and the code no longer throws on API 34 and higher. The branch is also up to date with main again.

@kkuan2011 kkuan2011 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved, assuming the build passes (Still running)

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