-
-
Notifications
You must be signed in to change notification settings - Fork 4
Added documentation for showing/hiding Alarm UI based on the flags #24
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update synchronizes the project version to 1.1.0 across documentation, workflow, and build files. It revises permission handling and scheduling examples to use coroutines, updates permission documentation (adding SYSTEM_ALERT_WINDOW), removes an explicit configuration property from the application initialization, and introduces a new tutorial on conditional alarm UI display. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeScreen (Composable)
participant CoroutineScope
participant PermissionState
participant AlarmScheduler
User->>HomeScreen (Composable): Clicks action button
HomeScreen (Composable)->>CoroutineScope: Launch coroutine
CoroutineScope->>PermissionState: Check all required permissions
alt Permissions granted
CoroutineScope->>AlarmScheduler: Schedule alarm (or perform action)
AlarmScheduler-->>CoroutineScope: Result (success/failure)
CoroutineScope-->>HomeScreen (Composable): Show toast (success/failure)
else Permissions not granted
CoroutineScope->>PermissionState: Request permissions
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (12)
docs/src/components/HomepageFeatures/index.tsx (1)
32-32: Fix typo in feature titleTypo reverses word order («…You That Don’t…») and reads awkwardly.
- title: 'Handles Permissions So You That Don’t Have To', + title: 'Handles Permissions So That You Don’t Have To',.github/ISSUE_TEMPLATE/bug_report.md (1)
26-30: Align list indentation to satisfy markdown-lint
markdownlintflags the leading space before each list item. Removing it keeps style consistent and silences the linter.- - Device: [e.g. Pixel 6] - - OS: [e.g. Android 13] - - TriggerX Library Version [e.g. 1.1.0] - - TriggerX Example App Version (if applicable) [e.g. 1.1] +- Device: [e.g. Pixel 6] +- OS: [e.g. Android 13] +- TriggerX Library Version [e.g. 1.1.0] +- TriggerX Example App Version (if applicable) [e.g. 1.1]docs/docs/tutorial-basics/1-installation.md (2)
16-18: Version string diverges from “latest-version” placeholder used elsewhereInstallation docs hard-code
1.1.0, whereas README now advertises the"latest-version"placeholder. Mixing approaches invites copy-paste errors and version skew.Consider switching to one consistent pattern (either hard-code the current release everywhere or use the placeholder everywhere).
47-49: Add a note about runtime overlay permission flow
SYSTEM_ALERT_WINDOWis a special permission that users must grant via the system settings panel opened withACTION_MANAGE_OVERLAY_PERMISSION; it is not requested through the normal permission dialog. A short hint here (or a link to the dedicated permission-handling section) would prevent confusion.docs/docs/tutorial-basics/4-permission-handling.mdx (1)
18-19: Minor naming/style polish
rememberCoroutineScope()is captured incoroutineScope, but other docs (e.g. README) use the variable namecoroutines. Pick one for consistency. Also preferlaunch {(space before{}) to match Kotlin style guides.docs/docs/tutorial-extras/showing-alarm-ui-conditionally.md (1)
42-45: Inconsistent flag naming
shouldShowAlarmActivityWhenDeviceIsActiveintroduces ashouldprefix whereas the previous flag omits it. Double-check the API for naming symmetry (showvsshouldShow) and update the snippet for coherence.README.md (3)
68-72: Placeholder vs fixed version inconsistencyREADME switched to
"latest-version"while installation docs pin1.1.0. Align both locations to prevent mismatch.
98-99: Overlay permission disclaimer missingSame remark as in installation docs: mention that
SYSTEM_ALERT_WINDOWneeds the user to enable it in system settings.
155-168: Variable name differs from other docsHere the coroutine scope is stored in
coroutines, elsewhere incoroutineScope. Harmonise to avoid mental friction.docs/docs/tutorial-basics/5-scheduling-alarm.md (1)
18-19: Consistent variable namingAgain, uses
coroutineScope(singular). Align across docs for uniformity.docs/docs/tutorial-extras/showing-dynamic-ui.md (2)
104-117: Avoid hard-codedalarmIdto prevent collisionsUsing a constant
1will overwrite previous alarms or DB rows. Persist the row-id returned by the insert (or generate a UUID) and feed that intoscheduleAlarm.-db.taskDao().insert(Task(1, "Meeting", "Discuss the roadmap")) +val taskId = db.taskDao().insert(Task(title = "Meeting", + description = "Discuss the roadmap")) ... -alarmId = 1 +alarmId = taskId.toInt()
118-120: CallrequestPermission()outside the coroutine
requestPermission()isn’t suspending; launching another coroutine adds unnecessary overhead and makes call-site intent less obvious.onClick = { - scope.launch { - if (permissionState.allRequiredGranted()) { - // … - } else { - permissionState.requestPermission() - } - } + if (permissionState.allRequiredGranted()) { + scope.launch { + // DB insert + alarm scheduling + } + } else { + permissionState.requestPermission() + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/docs/tutorial-extras/img/docsVersionDropdown.pngis excluded by!**/*.pngdocs/docs/tutorial-extras/img/localeDropdown.pngis excluded by!**/*.png
📒 Files selected for processing (12)
.github/ISSUE_TEMPLATE/bug_report.md(1 hunks).github/workflows/release.yaml(1 hunks)README.md(9 hunks)app/src/main/java/com/meticha/triggerxexample/TriggerXApplication.kt(0 hunks)docs/docs/tutorial-basics/1-installation.md(3 hunks)docs/docs/tutorial-basics/4-permission-handling.mdx(2 hunks)docs/docs/tutorial-basics/5-scheduling-alarm.md(2 hunks)docs/docs/tutorial-extras/showing-alarm-ui-conditionally.md(1 hunks)docs/docs/tutorial-extras/showing-dynamic-ui.md(1 hunks)docs/src/components/HomepageFeatures/index.tsx(1 hunks)settings.gradle.kts(1 hunks)triggerx/build.gradle.kts(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/meticha/triggerxexample/TriggerXApplication.kt
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.github/ISSUE_TEMPLATE/bug_report.md
29-29: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🔇 Additional comments (8)
settings.gradle.kts (1)
1-1: Suppression looks goodSuppressing
UnstableApiUsageat file level is reasonable for Gradle feature previews; no issues spotted..github/workflows/release.yaml (1)
9-11: Version bump propagated correctly
VERSION_NAMEis updated to 1.1.0 and flows through the later steps; workflow logic remains intact.triggerx/build.gradle.kts (1)
103-110: Maven coordinates & POM version updated consistentlyBoth the publication coordinates and
pom.versionnow point to 1.1.0—looks consistent with the workflow/env changes.docs/docs/tutorial-basics/4-permission-handling.mdx (1)
31-37: Good asynchronous flowWrapping the permission logic in
coroutineScope.launch { … }is the right call for non-blocking UI interactions. 👍docs/docs/tutorial-extras/showing-alarm-ui-conditionally.md (1)
24-27: Verify thatshowAlarmActivityWhenAppIsActivestill existsEarlier commits mention this flag being removed from the public API. If the property was indeed dropped, the sample will not compile.
- showAlarmActivityWhenAppIsActive = false + // showAlarmActivityWhenAppIsActive = false // <- confirm property name or update docsREADME.md (2)
25-26: Nice addition – direct docs linkLinking to the full documentation improves discoverability. 👍
167-174: Async scheduling snippet looks correctLaunch block correctly checks permissions, schedules, and surfaces result via toast. Solid example.
docs/docs/tutorial-basics/5-scheduling-alarm.md (1)
31-60: Good end-to-end exampleDemonstrates permission gating, scheduling, and user feedback all inside a coroutine – clear and correct.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores