Skip to content

feat: hover + focus states for desktop interaction#3

Open
ClankerGuru wants to merge 18 commits into
mainfrom
feature/hover-focus-states
Open

feat: hover + focus states for desktop interaction#3
ClankerGuru wants to merge 18 commits into
mainfrom
feature/hover-focus-states

Conversation

@ClankerGuru

@ClankerGuru ClankerGuru commented Mar 19, 2026

Copy link
Copy Markdown
Owner

Adds hover and focus interaction states to 23 interactive components, plus a shared InteractionModifiers.kt foundation module.

Changes

  • New: gort/foundation/InteractionModifiers.ktgortFocusRing, gortClickable, gortTextInput modifiers
  • 23 components updated: Button, IconButton, Checkbox, RadioButton, Toggle, Chip, TextField, TextArea, Slider, Dropdown, Tabs, SegmentedControl, Accordion, Breadcrumb, Pagination, ListItem, NavigationRail, Rating, NumberStepper, DatePicker, TimePicker, ChoiceGroup, CommandPalette
  • Hand cursor on clickable elements, text cursor on inputs
  • Bold 3dp focus ring (neobrutalist style)
  • Hover elevation via border color shifts

Verified

  • :gort:compileKotlinDesktop
  • :gort:compileKotlinWasmJs
  • :catalog:compileKotlinDesktop
  • :catalog:compileKotlinWasmJs

Summary by CodeRabbit

  • New Features

    • Enhanced hover, focus, cursor and focus-ring visuals across many components
    • Responsive Catalog app with Theme playground, live previews, and Android launcher
    • Seed-based palette generator, theme code export, and new typography presets
    • New showcase system for interactive component demos; scaffolded app layout, drawer, spinner, and marquee fades
  • Documentation

    • README expanded with full component catalog, theme system, platforms, and Catalog app instructions
  • Chores

    • CI/build and publishing workflow adjustments and artifact upload path updates

- maven-publish + signing plugins on :gort module
- POM with MIT license, SCM, developer info
- publishLibraryVariants(release) for Android
- Signing only when GPG_PASSPHRASE present (CI only)
- Release workflow publishes all KMP targets to Maven Central
- Empty javadoc JAR for Sonatype compliance
@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds pervasive hover/focus interaction support and shared interaction utilities to many UI components; introduces press-scale indication, seed-based color schemes, typography factories, responsive catalog app (including Android), a Theme playground, publishing/signing CI updates, and new showcase scaffolding and components.

Changes

Cohort / File(s) Summary
CI / Release Workflow
.github/workflows/release.yml
Set GRADLE_OPTS for build/publish, make test-report publishing conditional on JUnit XML presence, target :gort for Maven publish, and restrict release artifact uploads to */gort/build/libs/*.jar.
Gort Gradle / Publishing
gort/build.gradle.kts
Add maven-publish & signing; configure Maven Central publications, POM metadata, Sonatype creds, javadocJar, and conditional GPG signing; restrict Android publication to release variant; expose lucide lib.
README / Docs
README.md
Major README rewrite: badges, 52-component foundation-only description, Theme System, Platforms, Catalog App docs, full component/token catalogs, explicit dependencies, updated license attribution.
Catalog Android App
catalog/build.gradle.kts, catalog/src/androidMain/AndroidManifest.xml, catalog/src/androidMain/kotlin/.../MainActivity.kt
Add Android application plugin/target, activity-compose dependency, Android manifest and exported MainActivity wiring CatalogApp() as entry.
Catalog App Core & Theme Playground
catalog/src/commonMain/kotlin/.../CatalogApp.kt, catalog/src/commonMain/kotlin/.../ThemeScreen.kt
Introduce responsive CatalogApp with WindowSize, Masthead, NavRail/BottomNav variants; extend CatalogSection with icons and add ThemeScreen for live color/typography editing and callbacks.
Catalog Showcases & Screen Refactor
catalog/src/commonMain/kotlin/.../ComponentShowcase.kt, catalog/*Screen.kt
Add ComponentShowcase and ShowcaseSection; refactor multiple catalog screens (Buttons, Chat, Compound, Data, Display, Feedback, Inputs, Navigation) to use showcase wrappers with metadata, controls and code panels.
Foundation: Interaction Utilities
gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt
Add gortFocusRing, gortClickable, gortTextInput, and gortPressableShadow utilities to centralize hover/focus/press behaviors.
Foundation: Press Indication
gort/src/commonMain/kotlin/zone/clanker/gort/foundation/GortIndication.kt
Add GortIndication IndicationNodeFactory implementing press-scale animation and scaled drawing for press feedback.
Foundation: Responsive WindowSize
gort/src/commonMain/kotlin/zone/clanker/gort/foundation/WindowSize.kt
Add WindowSize enum, LocalWindowSize, and BoxWithConstraintsScope.currentWindowSize() helper for Compact/Medium/Expanded breakpoints.
Theme: Seed-based Color Scheme
gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortColorScheme.kt
Add GortColorScheme with fromSeed palette generation, preset factories (coral, ocean, forest, neon, mono, gazette) and HSL/blend/contrast helpers.
Typography Factories
gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortTypography.kt
Add GortTypography companion factory methods: monospace(), editorial(), sansSerif(), and withFonts(...) to create variant typographies.
Component Interaction Pattern
gort/src/commonMain/kotlin/zone/clanker/gort/components/*
Apply consistent interaction pattern: remember MutableInteractionSource, derive hover/focus via collectIsHoveredAsState()/collectIsFocusedAsState(), add .hoverable(...) and .pointerHoverIcon(...), update visuals (borders/backgrounds/shadows) across many components (Accordion, Breadcrumb, Button, Checkbox, Chip, ChoiceGroup, CommandPalette, DatePicker, Dropdown, IconButton, ListItem, NavigationRail, NumberStepper, Pagination, RadioButton, Rating, SegmentedControl, Slider, Tabs, TextField/TextArea, TimePicker, Toggle, etc.).
Component Tweaks & Helpers
gort/src/commonMain/kotlin/.../Chip.kt, Button.kt, IconButton.kt, Dropdown.kt, TextField.kt, Card.kt, Marquee.kt, Snackbar.kt
Add Color.compositeOver helper for Chip hover blending; three-state shadows & focus rings for Button/IconButton; shared interactionSources and hover visuals for Dropdown; dynamic border/hover behavior for TextField and extraction/removal of TextArea export from that file; Card padding to avoid shadow clipping; Marquee fade edges; Snackbar no longer forces full-width.
New UI Primitives
gort/src/commonMain/kotlin/zone/clanker/gort/components/GortDrawer.kt, GortScaffold.kt, Spinner.kt
Add GortDrawer (animated side drawer), GortScaffold (app scaffold with top/bottom bars and snackbar host), and Spinner (indeterminate circular spinner).
Catalog Proposal
opsx/changes/catalog-overhaul/proposal.md
Add a detailed proposal outlining the catalog redesign, visual language, roadmap and phased implementation (no code changes).
Version Catalog
gradle/libs.versions.toml
Add lucide = "com.composables:icons-lucide-cmp:2.2.1" library alias.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ThemeScreen
  participant CatalogApp
  participant GortTheme
  participant Components

  User->>ThemeScreen: select preset / pick seed / change typography
  ThemeScreen->>CatalogApp: onColorsChange(GortColors) / onTypographyChange(...)
  CatalogApp->>GortTheme: apply(colors, typography)
  GortTheme-->>Components: provide updated colors/typography (composition locals)
  Components-->>User: re-render previews (Button, Card, TextField, etc.)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

"I nibble on hover, I twitch at the focus light,
Badges gleam, Gradle hums through the night,
Catalog hops to Android, palettes bloom in seed,
I press, I scale, I publish — a rabbit's quiet deed,
Hop on, tiny changes, your UI's now freed!" 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.28% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: hover + focus states for desktop interaction' accurately captures the main focus of the PR, which adds hover and focus interaction states to 23+ components and introduces interaction modifiers.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/hover-focus-states

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🧹 Nitpick comments (13)
gort/build.gradle.kts (1)

87-98: Repository credentials are nullable; consider adding validation.

Using .orNull means username and password can be null. Publishing will fail at runtime with an unclear error if credentials are missing. This is acceptable if failures only occur in CI where secrets are expected, but consider adding a check or clearer error.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/build.gradle.kts` around lines 87 - 98, The maven repository credentials
in the repositories { maven { ... credentials { username =
providers.environmentVariable(...).orElse(...).orNull; password = ...orNull } }
} block can be null; add an explicit validation after obtaining the provider
values (e.g., check/requireNotNull or throw a GradleException) that validates
sonatypeUsername and sonatypePassword (the providers.environmentVariable(...)
orElse(providers.gradleProperty("sonatypeUsername")) and the corresponding
password provider) and emits a clear error message explaining which credential
is missing and that they must be set (use the provider keys
sonatypeUsername/sonatypePassword or the ORG_GRADLE_PROJECT_ env var names in
the message).
gort/src/commonMain/kotlin/zone/clanker/gort/components/ChoiceGroup.kt (1)

46-47: Use a stable key for remember inside the loop.

The remember call inside forEach should use the stable item.id as a key to ensure interaction sources remain correctly associated with their items if the list changes.

♻️ Proposed fix
-            val interactionSource = remember { MutableInteractionSource() }
+            val interactionSource = remember(item.id) { MutableInteractionSource() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/ChoiceGroup.kt`
around lines 46 - 47, The interaction source created inside the forEach loop
isn't stable; update the remember call to use the item's stable key so sources
stick to items when the list changes: change the creation of the interaction
source from remember { MutableInteractionSource() } to using the item's id as
the remember key (e.g., remember(item.id) { MutableInteractionSource() }), and
keep the subsequent use of collectIsHoveredAsState() unchanged so isHovered
remains bound to that stable MutableInteractionSource for each item.
gort/src/commonMain/kotlin/zone/clanker/gort/components/SegmentedControl.kt (1)

38-39: Use a stable key for remember inside the loop.

Same issue as in Tabs.kt: the remember call inside forEachIndexed lacks a key, which can cause interaction state misalignment if the options list changes.

♻️ Proposed fix
-            val interactionSource = remember { MutableInteractionSource() }
+            val interactionSource = remember(index) { MutableInteractionSource() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/SegmentedControl.kt`
around lines 38 - 39, In SegmentedControl, the remember {
MutableInteractionSource() } call inside the forEachIndexed over options can
misalign interaction state when options change; update the remember to use a
stable key (e.g., the option identity or index) so each option's
interactionSource is remembered by that stable key (refer to the forEachIndexed
loop, interactionSource variable, and collectIsHoveredAsState call) to ensure
consistent hover/interaction state when the options list mutates.
gort/src/commonMain/kotlin/zone/clanker/gort/components/Tabs.kt (1)

44-46: Use a stable key for remember inside the loop.

When remember is called inside forEachIndexed without a key, the interaction source instances can become misaligned if the tabs list changes (items added, removed, or reordered). This can cause hover/focus states to apply to the wrong tab.

♻️ Proposed fix to add a stable key
-                val interactionSource = remember { MutableInteractionSource() }
+                val interactionSource = remember(index) { MutableInteractionSource() }

Or, if tab titles are unique and stable:

-                val interactionSource = remember { MutableInteractionSource() }
+                val interactionSource = remember(title) { MutableInteractionSource() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Tabs.kt` around lines
44 - 46, The remember call for MutableInteractionSource inside the
forEachIndexed loop can become misaligned when the tabs list changes; update the
remember to use a stable key derived from the tab (e.g., a unique id or stable
title) so interactionSource is tied to a stable identifier rather than position
— replace remember { MutableInteractionSource() } with remember(key =
tab.someStableIdOrTitle) { MutableInteractionSource() } where tab is the loop
element used with collectIsHoveredAsState() / collectIsFocusedAsState().
gort/src/commonMain/kotlin/zone/clanker/gort/components/DatePicker.kt (1)

104-105: Use a stable key for remember inside nested loops.

The remember { MutableInteractionSource() } for day cells is inside nested loops without a key. Use the day value as a key to ensure stability across recompositions.

♻️ Proposed fix
-                            val dayInteraction = remember { MutableInteractionSource() }
+                            val dayInteraction = remember(day) { MutableInteractionSource() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/DatePicker.kt` around
lines 104 - 105, The remember call creating dayInteraction is unstable inside
nested loops; change remember { MutableInteractionSource() } to use a stable key
based on the loop day (e.g. remember(day) { MutableInteractionSource() }) so
each day cell retains its own InteractionSource across recompositions; update
the related isDayHovered binding (isDayHovered by
dayInteraction.collectIsHoveredAsState()) accordingly to reference the keyed
dayInteraction.
gort/src/commonMain/kotlin/zone/clanker/gort/components/Rating.kt (1)

37-38: Use a stable key for remember inside the loop.

The remember call inside repeat lacks a key, which can cause interaction sources to become misaligned if max changes or the component recomposes unexpectedly.

♻️ Proposed fix
-            val interactionSource = remember { MutableInteractionSource() }
+            val interactionSource = remember(index) { MutableInteractionSource() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Rating.kt` around
lines 37 - 38, The remember call that creates MutableInteractionSource inside
the repeat loop (used with interactionSource and isHovered via
collectIsHoveredAsState) needs a stable key to avoid misalignment when max or
recomposition changes; update the remember invocation inside the repeat to pass
a stable key (for example the loop index or a star identifier) so each star's
MutableInteractionSource is remembered by that key (e.g., remember(key) {
MutableInteractionSource() }) ensuring interactionSource and isHovered remain
correctly paired across recompositions.
gort/src/commonMain/kotlin/zone/clanker/gort/components/CommandPalette.kt (2)

109-125: Potential instability: MutableInteractionSource created inside items block.

The itemInteraction is created with remember {} inside the items lambda. While this works, remember without a key ties the state to the item's position in the list. If items are reordered or filtered, the interaction state may be associated with the wrong item.

Consider using remember(cmd.id) to key the interaction source to the item's identity:

♻️ Suggested improvement
                     items(filtered) { cmd ->
-                        val itemInteraction = remember { MutableInteractionSource() }
+                        val itemInteraction = remember(cmd.id) { MutableInteractionSource() }
                         val isItemHovered by itemInteraction.collectIsHoveredAsState()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/CommandPalette.kt`
around lines 109 - 125, The MutableInteractionSource is being remembered without
a stable key inside the items { cmd -> ... } lambda which can misassociate hover
state when the list is filtered/reordered; update the remember call that creates
itemInteraction to use a stable key derived from the item (e.g.,
remember(cmd.id) or another unique property on cmd) so the interaction source
stays tied to the item's identity; keep using
itemInteraction.collectIsHoveredAsState() and the existing Row
clickable/hoverable logic unchanged except for the keyed remember.

74-74: Consider adding a comment explaining the empty clickable.

The clickable(enabled = false) {} pattern to prevent click-through to the backdrop is non-obvious. A brief comment would improve readability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/CommandPalette.kt` at
line 74, Add a short inline comment above or beside the empty clickable(enabled
= false) {} in CommandPalette.kt to explain its purpose (preventing
click-through to the backdrop when the palette is open); reference the clickable
call in the CommandPalette composable so future readers understand the no-op
lambda is intentional and not a forgotten handler.
README.md (2)

58-75: Add blank lines around tables for markdown consistency.

Markdownlint (MD058) flags that tables should be surrounded by blank lines. This applies to all component tables throughout the document.

📝 Example fix for Core Input table
 ### Core Input
+
 | Component | Description |
 |-----------|-------------|
 ...
 | `ColorPicker` | Color selection with palette |
+
 ### Display
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 58 - 75, Add blank lines before and after the
markdown tables (e.g., the "Core Input" table under the heading) so each table
is surrounded by an empty line as required by markdownlint MD058; update every
component table in the README.md (including tables listing Button, IconButton,
TextField, etc.) to have a single blank line above the opening |--- row and a
single blank line after the final row.

229-233: Add language specifier to fenced code block.

The dependencies code block is missing a language identifier, which triggers markdownlint MD040.

📝 Suggested fix
-```
+```text
 compose.foundation
 compose.runtime
 compose.ui
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @README.md around lines 229 - 233, The fenced code block listing dependencies
lacks a language specifier, causing a markdownlint MD040 warning. Add the
language specifier "text" immediately after the opening triple backticks in the
fenced code block containing compose.foundation, compose.runtime, and compose.ui
to resolve this.


</details>

</blockquote></details>
<details>
<summary>gort/src/commonMain/kotlin/zone/clanker/gort/components/Dropdown.kt (1)</summary><blockquote>

`101-107`: **Hover overrides selected state visually.**

The `when` expression checks `isItemHovered` before `item == selectedItem`, so a hovered selected item shows the hover color (alpha 0.5) instead of the solid selected color. This may cause visual inconsistency where the selected item appears to "dim" on hover.

Consider prioritizing selected state or combining them:


<details>
<summary>♻️ Option: Prioritize selected state</summary>

```diff
                                 .background(
                                     when {
+                                        item == selectedItem -> colors.primaryContainer
                                         isItemHovered -> colors.primaryContainer.copy(alpha = 0.5f)
-                                        item == selectedItem -> colors.primaryContainer
                                         else -> colors.surface
                                     },
                                 )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Dropdown.kt` around
lines 101 - 107, The hover branch currently takes precedence over the selected
state in the background when-expression causing a selected item to show the
hover (dimmed) color; update the when-expression in the Dropdown background
logic so it checks item == selectedItem before isItemHovered (or otherwise merge
the cases) so selectedItem uses the solid colors.primaryContainer regardless of
hover; look for the when-expression using isItemHovered, selectedItem and
colors.primaryContainer in Dropdown.kt and reorder or combine those conditions
accordingly.
gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt (1)

39-52: Add enabled support to shared interaction helpers.

gortClickable / gortTextInput always apply hover and pointer icons. Adding an enabled parameter here centralizes disabled-state behavior and prevents per-component drift.

Suggested refactor
 fun Modifier.gortClickable(
     interactionSource: MutableInteractionSource,
-): Modifier = this
-    .hoverable(interactionSource)
-    .pointerHoverIcon(PointerIcon.Hand)
+    enabled: Boolean = true,
+): Modifier = if (enabled) {
+    this
+        .hoverable(interactionSource)
+        .pointerHoverIcon(PointerIcon.Hand)
+} else {
+    this
+}
@@
 fun Modifier.gortTextInput(
     interactionSource: MutableInteractionSource,
-): Modifier = this
-    .hoverable(interactionSource)
-    .pointerHoverIcon(PointerIcon.Text)
+    enabled: Boolean = true,
+): Modifier = if (enabled) {
+    this
+        .hoverable(interactionSource)
+        .pointerHoverIcon(PointerIcon.Text)
+} else {
+    this
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt`
around lines 39 - 52, Add an enabled boolean to gortClickable and gortTextInput
signatures and use it to gate hover and pointer icon behavior: update functions
gortClickable(interactionSource: MutableInteractionSource, enabled: Boolean =
true) and gortTextInput(interactionSource: MutableInteractionSource, enabled:
Boolean = true) to call hoverable with the enabled flag
(hoverable(interactionSource, enabled = enabled)) and only apply
pointerHoverIcon(PointerIcon.Hand/Text) when enabled is true (otherwise return
the Modifier without the pointerHoverIcon); keep existing function names and
parameters to minimize call-site churn by providing a default enabled=true.
gort/src/commonMain/kotlin/zone/clanker/gort/components/ListItem.kt (1)

38-44: Non-clickable rows can skip hoverable.

When onClick == null, hover state is unused and can be removed to keep interaction plumbing minimal.

Suggested simplification
             .then(
                 if (isClickable) Modifier
                     .hoverable(interactionSource)
                     .pointerHoverIcon(PointerIcon.Hand)
                     .clickable(onClick = onClick)
-                else Modifier.hoverable(interactionSource)
+                else Modifier
             )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/ListItem.kt` around
lines 38 - 44, The modifier chain currently applies hoverable(interactionSource)
even when not clickable; update the conditional in ListItem.kt (the .then( ... )
block using isClickable/onClick/interactionSource) so that when onClick is null
(i.e., not clickable) you return plain Modifier (not Modifier.hoverable(...)),
and only attach hoverable(interactionSource),
pointerHoverIcon(PointerIcon.Hand), and clickable(onClick = onClick) in the
clickable branch; remove the unused hoverable usage from the non-clickable
branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/release.yml:
- Around line 48-49: The GRADLE_OPTS assignment is splitting JVM args because
the space separates -XX:... from the -Dorg.gradle.jvmargs value; update how JVM
args are passed so both -Xmx4g and -XX:MaxMetaspaceSize=512m are
quoted/contained inside the org.gradle.jvmargs property (i.e., set GRADLE_OPTS
so that org.gradle.jvmargs receives a single string containing both flags), or
alternatively move the setting into gradle.properties as org.gradle.jvmargs or
use JAVA_TOOL_OPTIONS to apply the JVM flags; adjust the environment entry that
sets GRADLE_OPTS accordingly so -XX:MaxMetaspaceSize=512m is not treated as a
separate wrapper JVM argument.

In `@gort/build.gradle.kts`:
- Around line 101-107: The signing block reads gpgPassphrase but never
configures it, so update the signing configuration to actually provide the
passphrase instead of only calling useGpgCmd(); either export the Gradle
property signing.gnupg.passphrase in CI or switch to in-memory signing by using
the signing API (e.g., configure signing.signingKey / signing.password from the
gpgPassphrase or related env vars) and then call sign(publishing.publications);
locate the signing block and the gpgPassphrase variable in the file (the code
that calls useGpgCmd() and sign(publishing.publications)) and wire the
passphrase into the signing setup used by your CI strategy.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Breadcrumb.kt`:
- Around line 43-57: Clickable breadcrumb items only change on hover and lack a
keyboard focus-visible cue; update the clickable branch (the isClickable block
where you create MutableInteractionSource, isHovered via
interactionSource.collectIsHoveredAsState(), and render BasicText with
Modifier.hoverable/.pointerHoverIcon/.clickable(item.onClick)) to also observe
focus and make the item focusable: create isFocused by calling
interactionSource.collectIsFocusedAsState(), pass the same interactionSource
into clickable (clickable(onClick = item.onClick, interactionSource =
interactionSource)) and add Modifier.focusable(interactionSource =
interactionSource) (or focusable()) to the modifier chain, then adjust the text
style to show a visible focus state (e.g., change textDecoration or color when
isFocused || isHovered) so keyboard users get a clear focus-visible treatment.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Checkbox.kt`:
- Line 39: The Checkbox composable is missing a focusable modifier so
interactionSource.collectIsFocusedAsState() can ever become true; update the
modifier chain on the Checkbox root (where interactionSource is created and
isFocused is derived) to insert .focusable(interactionSource =
interactionSource) between the existing .hoverable(...) and .clickable(...)
modifiers so keyboard focus works and the focus visual (3dp border, primary
color) can be reached.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Chip.kt`:
- Around line 38-39: The Chip composable currently applies
.pointerHoverIcon(PointerIcon.Hand) and .hoverable(interactionSource) but has no
onClick, which misleads users; either remove the pointerHoverIcon call for
display-only chips or add an optional onClick parameter to Chip and only apply
Modifier.clickable(...) and .pointerHoverIcon(PointerIcon.Hand) when onClick is
non-null (leave hoverable if needed for ripple/hover visuals). Update the Chip
function signature to accept onClick: (() -> Unit)? = null and conditionally
attach Modifier.clickable(onClick = onClick) and pointerHoverIcon only when
onClick != null, otherwise drop the hand cursor.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/DatePicker.kt`:
- Around line 54-58: The month navigation arrow modifiers in DatePicker use
pointerHoverIcon(...) then clickable(...) but are missing
hoverable(interactionSource); update both arrow modifier chains (the left arrow
using PointerIcon.Hand and the right arrow) to follow the pattern
hoverable(interactionSource) → pointerHoverIcon(...) → clickable(...). If an
interactionSource isn't already present in the DatePicker scope, create one
(e.g., remember { MutableInteractionSource() }) and use it as the argument to
hoverable(interactionSource).

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/RadioButton.kt`:
- Around line 36-40: The radio button still shows hover/focus affordances when
enabled == false; update the logic in the RadioButton composable so all
interactive visuals and pointer behavior are gated by the enabled flag: compute
borderColor using enabled && isFocused / enabled && isHovered (so defaults to
colors.border when disabled), do not set the hand pointerIcon or pointer hover
handlers when enabled is false, and ensure any isHovered state is
cleared/ignored when enabled == false (update the modifier/hover handlers and
pointerIcon usage where isHovered, isFocused, borderColor, pointerIcon, and
enabled are referenced). This will prevent hover/focus visuals and the hand
cursor for disabled controls.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Rating.kt`:
- Around line 40-41: The direct mutation of hoveredIndex during composition
(inside the branch using isHovered and index) must be moved into a Compose
side-effect: wrap the state update in a LaunchedEffect keyed by isHovered and
index (or use a distinct key like index when isHovered changes) so the
assignment to hoveredIndex happens inside the side-effect rather than during
composition; update the import to include
androidx.compose.runtime.LaunchedEffect and replace the direct assignments to
hoveredIndex with a LaunchedEffect block that sets hoveredIndex = index when
isHovered is true and sets hoveredIndex = -1 when the key indicates hover ended.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/TextArea.kt`:
- Around line 42-53: The TextArea currently uses isFocused/isHovered and always
applies hoverable(), pointerHoverIcon(PointerIcon.Text), and reactive border
styling; update the Box modifier and border logic in TextArea (the variables
borderColor, borderWidth, and the modifier chain that includes hoverable,
pointerHoverIcon, and border) to respect the enabled flag: when enabled is
false, do not attach hoverable(interactionSource) or pointerHoverIcon(Text),
force a neutral borderColor/borderWidth (e.g., use colors.disabled or
colors.border and borders.default) and ignore isFocused/isHovered so the visual
and cursor remain neutral for disabled inputs.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/TextField.kt`:
- Around line 42-68: Replace the duplicated hover/cursor modifiers in TextField
by removing the .hoverable(interactionSource) and
.pointerHoverIcon(PointerIcon.Text) calls and instead apply the shared extension
.gortTextInput(interactionSource) on the modifier; keep the existing
interactionSource, collectIsFocusedAsState/collectIsHoveredAsState usage and
border styling in TextField (and do the same refactor in TextArea) so
cursor/hover behavior is centralized in gortTextInput while component-specific
state and styling remain unchanged.
- Around line 46-59: The hover affordances are currently active even when the
TextField is disabled; update the BasicTextField modifiers so hoverable uses the
component's enabled flag and only add pointerHoverIcon(PointerIcon.Text) when
enabled is true. Specifically, in the BasicTextField modifier chain (in
TextField.kt) change hoverable(interactionSource) to hoverable(enabled =
enabled, interactionSource = interactionSource) and wrap or conditionally
include pointerHoverIcon(PointerIcon.Text) so it is only applied when enabled is
true, matching the approach used in Pagination and NumberStepper.

In `@README.md`:
- Line 56: The section header "## Components (52)" is inconsistent with the
actual component list; update the header string in README.md (the "## Components
(52)" line) to reflect the true count of components by replacing "(52)" with the
actual number that matches the list below so the header and the component list
are consistent.
- Line 9: The README's component count string "**52 components.**" is incorrect;
reconcile it with the actual component list (Core Input, Display, Feedback,
Navigation, Data & Compound, Chat, Foundation) and update the count to the
correct number (57) or adjust the listed components to match 52; locate the
sentence containing "52 components." in README.md and replace it with the
accurate count and/or update the category tables so the total matches the stated
number.

---

Nitpick comments:
In `@gort/build.gradle.kts`:
- Around line 87-98: The maven repository credentials in the repositories {
maven { ... credentials { username =
providers.environmentVariable(...).orElse(...).orNull; password = ...orNull } }
} block can be null; add an explicit validation after obtaining the provider
values (e.g., check/requireNotNull or throw a GradleException) that validates
sonatypeUsername and sonatypePassword (the providers.environmentVariable(...)
orElse(providers.gradleProperty("sonatypeUsername")) and the corresponding
password provider) and emits a clear error message explaining which credential
is missing and that they must be set (use the provider keys
sonatypeUsername/sonatypePassword or the ORG_GRADLE_PROJECT_ env var names in
the message).

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/ChoiceGroup.kt`:
- Around line 46-47: The interaction source created inside the forEach loop
isn't stable; update the remember call to use the item's stable key so sources
stick to items when the list changes: change the creation of the interaction
source from remember { MutableInteractionSource() } to using the item's id as
the remember key (e.g., remember(item.id) { MutableInteractionSource() }), and
keep the subsequent use of collectIsHoveredAsState() unchanged so isHovered
remains bound to that stable MutableInteractionSource for each item.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/CommandPalette.kt`:
- Around line 109-125: The MutableInteractionSource is being remembered without
a stable key inside the items { cmd -> ... } lambda which can misassociate hover
state when the list is filtered/reordered; update the remember call that creates
itemInteraction to use a stable key derived from the item (e.g.,
remember(cmd.id) or another unique property on cmd) so the interaction source
stays tied to the item's identity; keep using
itemInteraction.collectIsHoveredAsState() and the existing Row
clickable/hoverable logic unchanged except for the keyed remember.
- Line 74: Add a short inline comment above or beside the empty
clickable(enabled = false) {} in CommandPalette.kt to explain its purpose
(preventing click-through to the backdrop when the palette is open); reference
the clickable call in the CommandPalette composable so future readers understand
the no-op lambda is intentional and not a forgotten handler.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/DatePicker.kt`:
- Around line 104-105: The remember call creating dayInteraction is unstable
inside nested loops; change remember { MutableInteractionSource() } to use a
stable key based on the loop day (e.g. remember(day) {
MutableInteractionSource() }) so each day cell retains its own InteractionSource
across recompositions; update the related isDayHovered binding (isDayHovered by
dayInteraction.collectIsHoveredAsState()) accordingly to reference the keyed
dayInteraction.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Dropdown.kt`:
- Around line 101-107: The hover branch currently takes precedence over the
selected state in the background when-expression causing a selected item to show
the hover (dimmed) color; update the when-expression in the Dropdown background
logic so it checks item == selectedItem before isItemHovered (or otherwise merge
the cases) so selectedItem uses the solid colors.primaryContainer regardless of
hover; look for the when-expression using isItemHovered, selectedItem and
colors.primaryContainer in Dropdown.kt and reorder or combine those conditions
accordingly.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/ListItem.kt`:
- Around line 38-44: The modifier chain currently applies
hoverable(interactionSource) even when not clickable; update the conditional in
ListItem.kt (the .then( ... ) block using isClickable/onClick/interactionSource)
so that when onClick is null (i.e., not clickable) you return plain Modifier
(not Modifier.hoverable(...)), and only attach hoverable(interactionSource),
pointerHoverIcon(PointerIcon.Hand), and clickable(onClick = onClick) in the
clickable branch; remove the unused hoverable usage from the non-clickable
branch.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Rating.kt`:
- Around line 37-38: The remember call that creates MutableInteractionSource
inside the repeat loop (used with interactionSource and isHovered via
collectIsHoveredAsState) needs a stable key to avoid misalignment when max or
recomposition changes; update the remember invocation inside the repeat to pass
a stable key (for example the loop index or a star identifier) so each star's
MutableInteractionSource is remembered by that key (e.g., remember(key) {
MutableInteractionSource() }) ensuring interactionSource and isHovered remain
correctly paired across recompositions.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/SegmentedControl.kt`:
- Around line 38-39: In SegmentedControl, the remember {
MutableInteractionSource() } call inside the forEachIndexed over options can
misalign interaction state when options change; update the remember to use a
stable key (e.g., the option identity or index) so each option's
interactionSource is remembered by that stable key (refer to the forEachIndexed
loop, interactionSource variable, and collectIsHoveredAsState call) to ensure
consistent hover/interaction state when the options list mutates.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Tabs.kt`:
- Around line 44-46: The remember call for MutableInteractionSource inside the
forEachIndexed loop can become misaligned when the tabs list changes; update the
remember to use a stable key derived from the tab (e.g., a unique id or stable
title) so interactionSource is tied to a stable identifier rather than position
— replace remember { MutableInteractionSource() } with remember(key =
tab.someStableIdOrTitle) { MutableInteractionSource() } where tab is the loop
element used with collectIsHoveredAsState() / collectIsFocusedAsState().

In
`@gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt`:
- Around line 39-52: Add an enabled boolean to gortClickable and gortTextInput
signatures and use it to gate hover and pointer icon behavior: update functions
gortClickable(interactionSource: MutableInteractionSource, enabled: Boolean =
true) and gortTextInput(interactionSource: MutableInteractionSource, enabled:
Boolean = true) to call hoverable with the enabled flag
(hoverable(interactionSource, enabled = enabled)) and only apply
pointerHoverIcon(PointerIcon.Hand/Text) when enabled is true (otherwise return
the Modifier without the pointerHoverIcon); keep existing function names and
parameters to minimize call-site churn by providing a default enabled=true.

In `@README.md`:
- Around line 58-75: Add blank lines before and after the markdown tables (e.g.,
the "Core Input" table under the heading) so each table is surrounded by an
empty line as required by markdownlint MD058; update every component table in
the README.md (including tables listing Button, IconButton, TextField, etc.) to
have a single blank line above the opening |--- row and a single blank line
after the final row.
- Around line 229-233: The fenced code block listing dependencies lacks a
language specifier, causing a markdownlint MD040 warning. Add the language
specifier "text" immediately after the opening triple backticks in the fenced
code block containing compose.foundation, compose.runtime, and compose.ui to
resolve this.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fcb0713b-4867-400b-bcf2-f473f8b334a8

📥 Commits

Reviewing files that changed from the base of the PR and between 991a8db and eccfdea.

📒 Files selected for processing (27)
  • .github/workflows/release.yml
  • README.md
  • gort/build.gradle.kts
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Accordion.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Breadcrumb.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Button.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Checkbox.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Chip.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/ChoiceGroup.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/CommandPalette.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/DatePicker.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Dropdown.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/IconButton.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/ListItem.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/NavigationRail.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/NumberStepper.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Pagination.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/RadioButton.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Rating.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/SegmentedControl.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Slider.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Tabs.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/TextArea.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/TextField.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/TimePicker.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Toggle.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt

Comment on lines +48 to +49
env:
GRADLE_OPTS: "-Dorg.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=512m"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

GRADLE_OPTS syntax causes JVM args to be split incorrectly.

The space in -Dorg.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=512m causes -XX:MaxMetaspaceSize=512m to be parsed as a separate argument to the Gradle wrapper JVM rather than part of org.gradle.jvmargs for the daemon.

🔧 Proposed fix: Set GRADLE_OPTS directly or use gradle.properties

Option 1: Use GRADLE_OPTS directly for wrapper JVM args

         env:
-          GRADLE_OPTS: "-Dorg.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=512m"
+          GRADLE_OPTS: "-Xmx4g -XX:MaxMetaspaceSize=512m"

Option 2: Use JAVA_TOOL_OPTIONS for broader coverage

         env:
-          GRADLE_OPTS: "-Dorg.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=512m"
+          JAVA_TOOL_OPTIONS: "-Xmx4g -XX:MaxMetaspaceSize=512m"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
env:
GRADLE_OPTS: "-Dorg.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=512m"
env:
GRADLE_OPTS: "-Xmx4g -XX:MaxMetaspaceSize=512m"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release.yml around lines 48 - 49, The GRADLE_OPTS
assignment is splitting JVM args because the space separates -XX:... from the
-Dorg.gradle.jvmargs value; update how JVM args are passed so both -Xmx4g and
-XX:MaxMetaspaceSize=512m are quoted/contained inside the org.gradle.jvmargs
property (i.e., set GRADLE_OPTS so that org.gradle.jvmargs receives a single
string containing both flags), or alternatively move the setting into
gradle.properties as org.gradle.jvmargs or use JAVA_TOOL_OPTIONS to apply the
JVM flags; adjust the environment entry that sets GRADLE_OPTS accordingly so
-XX:MaxMetaspaceSize=512m is not treated as a separate wrapper JVM argument.

Comment thread gort/build.gradle.kts
Comment on lines +101 to +107
signing {
val gpgPassphrase = providers.environmentVariable("GPG_PASSPHRASE").orNull
if (gpgPassphrase != null) {
useGpgCmd()
}
sign(publishing.publications)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Gradle signing plugin useGpgCmd passphrase configuration

💡 Result:

To configure the passphrase for Gradle's signing plugin when using useGpgCmd, set the signing.gnupg.passphrase property in gradle.properties. This provides the passphrase directly to the gpg command, bypassing the need for gpg-agent interactive prompts. Example gradle.properties: signing.gnupg.executable=gpg signing.gnupg.keyName=your-key-id signing.gnupg.passphrase=your-passphrase In build.gradle (Groovy): signing { useGpgCmd sign publishing.publications.mavenJava // or whatever publication } Key points: - Defaults to gpg2 executable unless signing.gnupg.executable=gpg is set. - For GnuPG 2.x (common), omit or set signing.gnupg.useLegacyGpg=false (default). - If unspecified, falls back to gpg-agent for passphrase. - Can also pass via CLI: -Psigning.gnupg.passphrase=your-passphrase. - Official docs confirm this configuration for GnupgSignatory used by useGpgCmd.

Citations:


🏁 Script executed:

# First, find the build.gradle.kts file mentioned in the review
find . -name "build.gradle.kts" -type f | head -20

Repository: ClankerGuru/gort

Length of output: 129


🏁 Script executed:

# Read the gort/build.gradle.kts file to see the full signing configuration context
wc -l gort/build.gradle.kts

Repository: ClankerGuru/gort

Length of output: 85


🏁 Script executed:

# Read the entire gort/build.gradle.kts file to see the complete signing configuration
cat -n gort/build.gradle.kts

Repository: ClankerGuru/gort

Length of output: 4117


GPG passphrase is captured but not configured for signing.

The gpgPassphrase variable is read from the environment but never passed to the signing configuration. When using useGpgCmd(), the passphrase must be provided via the signing.gnupg.passphrase Gradle property or pre-cached in gpg-agent. In CI/CD environments, this configuration will likely fail.

Either set the Gradle property in your workflow:

env:
  ORG_GRADLE_PROJECT_signing.gnupg.passphrase: ${{ secrets.GPG_PASSPHRASE }}

Or use in-memory signing which handles the passphrase directly:

 signing {
-    val gpgPassphrase = providers.environmentVariable("GPG_PASSPHRASE").orNull
-    if (gpgPassphrase != null) {
-        useGpgCmd()
-    }
+    val signingKey = providers.environmentVariable("GPG_SIGNING_KEY").orNull
+    val signingPassword = providers.environmentVariable("GPG_PASSPHRASE").orNull
+    if (signingKey != null && signingPassword != null) {
+        useInMemoryPgpKeys(signingKey, signingPassword)
+    }
     sign(publishing.publications)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/build.gradle.kts` around lines 101 - 107, The signing block reads
gpgPassphrase but never configures it, so update the signing configuration to
actually provide the passphrase instead of only calling useGpgCmd(); either
export the Gradle property signing.gnupg.passphrase in CI or switch to in-memory
signing by using the signing API (e.g., configure signing.signingKey /
signing.password from the gpgPassphrase or related env vars) and then call
sign(publishing.publications); locate the signing block and the gpgPassphrase
variable in the file (the code that calls useGpgCmd() and
sign(publishing.publications)) and wire the passphrase into the signing setup
used by your CI strategy.

Comment on lines +43 to +57
if (isClickable) {
val interactionSource = remember { MutableInteractionSource() }
val isHovered by interactionSource.collectIsHoveredAsState()

BasicText(
text = item.label,
style = Gort.typography.body.copy(
color = colors.primary,
textDecoration = if (isHovered) TextDecoration.Underline else TextDecoration.None,
),
modifier = Modifier
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Hand)
.clickable(onClick = item.onClick),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clickable breadcrumb items need a focus-visible state.

Current styling changes only react to hover. Keyboard users won’t get a visible focus cue on interactive crumbs.

Suggested fix
+import androidx.compose.foundation.interaction.collectIsFocusedAsState
@@
                 val interactionSource = remember { MutableInteractionSource() }
                 val isHovered by interactionSource.collectIsHoveredAsState()
+                val isFocused by interactionSource.collectIsFocusedAsState()
@@
                     style = Gort.typography.body.copy(
                         color = colors.primary,
-                        textDecoration = if (isHovered) TextDecoration.Underline else TextDecoration.None,
+                        textDecoration = if (isHovered || isFocused) TextDecoration.Underline else TextDecoration.None,
                     ),
                     modifier = Modifier
                         .hoverable(interactionSource)
                         .pointerHoverIcon(PointerIcon.Hand)
-                        .clickable(onClick = item.onClick),
+                        .clickable(
+                            interactionSource = interactionSource,
+                            indication = null,
+                            onClick = item.onClick,
+                        ),
                 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (isClickable) {
val interactionSource = remember { MutableInteractionSource() }
val isHovered by interactionSource.collectIsHoveredAsState()
BasicText(
text = item.label,
style = Gort.typography.body.copy(
color = colors.primary,
textDecoration = if (isHovered) TextDecoration.Underline else TextDecoration.None,
),
modifier = Modifier
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Hand)
.clickable(onClick = item.onClick),
)
if (isClickable) {
val interactionSource = remember { MutableInteractionSource() }
val isHovered by interactionSource.collectIsHoveredAsState()
val isFocused by interactionSource.collectIsFocusedAsState()
BasicText(
text = item.label,
style = Gort.typography.body.copy(
color = colors.primary,
textDecoration = if (isHovered || isFocused) TextDecoration.Underline else TextDecoration.None,
),
modifier = Modifier
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Hand)
.clickable(
interactionSource = interactionSource,
indication = null,
onClick = item.onClick,
),
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Breadcrumb.kt` around
lines 43 - 57, Clickable breadcrumb items only change on hover and lack a
keyboard focus-visible cue; update the clickable branch (the isClickable block
where you create MutableInteractionSource, isHovered via
interactionSource.collectIsHoveredAsState(), and render BasicText with
Modifier.hoverable/.pointerHoverIcon/.clickable(item.onClick)) to also observe
focus and make the item focusable: create isFocused by calling
interactionSource.collectIsFocusedAsState(), pass the same interactionSource
into clickable (clickable(onClick = item.onClick, interactionSource =
interactionSource)) and add Modifier.focusable(interactionSource =
interactionSource) (or focusable()) to the modifier chain, then adjust the text
style to show a visible focus state (e.g., change textDecoration or color when
isFocused || isHovered) so keyboard users get a clear focus-visible treatment.

val shape = Gort.corners.small
val interactionSource = remember { MutableInteractionSource() }
val isHovered by interactionSource.collectIsHoveredAsState()
val isFocused by interactionSource.collectIsFocusedAsState()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Compose Multiplatform clickable focusable keyboard focus interaction source

💡 Result:

In Compose Multiplatform, create clickable and focusable (keyboard-navigable) interactions using Modifier.clickable combined with Modifier.focusable. This enables both touch/mouse clicks and keyboard navigation (Tab, arrows, Enter to activate). Key concepts: - Modifier.focusable makes a composable participate in focus traversal and receive keyboard input. - Modifier.clickable handles pointer interactions (clicks). - Use FocusRequester for programmatic focus control. - onKeyEvent or onPreviewKeyEvent for custom key handling (e.g., Enter to trigger action). - Focus groups (focusGroup) control traversal order. Example for a focusable, clickable button: import androidx.compose.foundation.background import androidx.compose.foundation.focusable import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.interaction.collectIsFocusedAsState import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.padding import androidx.compose.material.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.remember import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.Color import androidx.compose.ui.input.key.* import androidx.compose.ui.unit.dp @OptIn(ExperimentalComposeUiApi::class) @Composable fun FocusableClickableExample { val focusRequester = remember { FocusRequester } val interactionSource = remember { MutableInteractionSource } val isFocused = interactionSource.collectIsFocusedAsState.value Column { Box( modifier = Modifier .focusRequester(focusRequester) .focusable(interactionSource = interactionSource) .background(if (isFocused) Color.Blue else Color.Gray) .clickable { /* Click action */ println("Clicked!") } .onKeyEvent { if (it.type == KeyEventType.KeyDown && it.key == Key.Enter) { println("Activated by Enter!") true } else false } .padding(16.dp) ) { Text("Focusable & Clickable", color = Color.White) } } } Usage notes: - Chain modifiers: focusRequester -> focusable -> clickable -> onKeyEvent. - Visual feedback: Use collectIsFocusedAsState to style focused state. - Keyboard navigation works out-of-box (Tab/Shift-Tab, arrows). - For desktop: onPreviewKeyEvent for shortcuts (Result 2). - Accessibility: Add semantics { role = Role.Button; contentDescription = "..." } (Result 11,24). - Platforms: Works across Android/Desktop/iOS/Web; desktop has full keyboard support (Results 2,16,17). Official sources confirm focusable enables keyboard focus for custom composables (Results 3,5,16,17). Buttons/TextFields are focusable by default.

Citations:


🏁 Script executed:

cat -n gort/src/commonMain/kotlin/zone/clanker/gort/components/Checkbox.kt

Repository: ClankerGuru/gort

Length of output: 3414


Add focusable() modifier to enable keyboard focus.

collectIsFocusedAsState() requires the element to be focusable. The current modifier chain uses clickable and hoverable but lacks .focusable(interactionSource = interactionSource). Without it, the element cannot receive keyboard focus, and isFocused will never be true, making the focus visual feedback (3dp border, primary color) unreachable via keyboard navigation. Add .focusable(interactionSource = interactionSource) between hoverable and clickable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Checkbox.kt` at line
39, The Checkbox composable is missing a focusable modifier so
interactionSource.collectIsFocusedAsState() can ever become true; update the
modifier chain on the Checkbox root (where interactionSource is created and
isFocused is derived) to insert .focusable(interactionSource =
interactionSource) between the existing .hoverable(...) and .clickable(...)
modifiers so keyboard focus works and the focus visual (3dp border, primary
color) can be reached.

Comment thread gort/src/commonMain/kotlin/zone/clanker/gort/components/Chip.kt Outdated
Comment on lines +42 to +53
val borderColor = when {
isFocused -> colors.primary
isHovered -> colors.primary.copy(alpha = 0.6f)
else -> colors.border
}
val borderWidth = if (isFocused) borders.heavy else borders.default

Box(
modifier = modifier
.border(borders.default, colors.border, Gort.corners.default)
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Text)
.border(borderWidth, borderColor, Gort.corners.default)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Disabled TextArea still behaves like an active input.

Line 51–53 apply hover + text cursor regardless of enabled, and border color still reacts to hover/focus. Disabled input should keep neutral visuals/cursor.

Suggested fix
     val borderColor = when {
+        !enabled -> colors.border
         isFocused -> colors.primary
         isHovered -> colors.primary.copy(alpha = 0.6f)
         else -> colors.border
     }
@@
         modifier = modifier
-            .hoverable(interactionSource)
-            .pointerHoverIcon(PointerIcon.Text)
+            .then(
+                if (enabled) {
+                    Modifier
+                        .hoverable(interactionSource)
+                        .pointerHoverIcon(PointerIcon.Text)
+                } else {
+                    Modifier
+                }
+            )
             .border(borderWidth, borderColor, Gort.corners.default)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val borderColor = when {
isFocused -> colors.primary
isHovered -> colors.primary.copy(alpha = 0.6f)
else -> colors.border
}
val borderWidth = if (isFocused) borders.heavy else borders.default
Box(
modifier = modifier
.border(borders.default, colors.border, Gort.corners.default)
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Text)
.border(borderWidth, borderColor, Gort.corners.default)
val borderColor = when {
!enabled -> colors.border
isFocused -> colors.primary
isHovered -> colors.primary.copy(alpha = 0.6f)
else -> colors.border
}
val borderWidth = if (isFocused) borders.heavy else borders.default
Box(
modifier = modifier
.then(
if (enabled) {
Modifier
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Text)
} else {
Modifier
}
)
.border(borderWidth, borderColor, Gort.corners.default)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/TextArea.kt` around
lines 42 - 53, The TextArea currently uses isFocused/isHovered and always
applies hoverable(), pointerHoverIcon(PointerIcon.Text), and reactive border
styling; update the Box modifier and border logic in TextArea (the variables
borderColor, borderWidth, and the modifier chain that includes hoverable,
pointerHoverIcon, and border) to respect the enabled flag: when enabled is
false, do not attach hoverable(interactionSource) or pointerHoverIcon(Text),
force a neutral borderColor/borderWidth (e.g., use colors.disabled or
colors.border and borders.default) and ignore isFocused/isHovered so the visual
and cursor remain neutral for disabled inputs.

Comment on lines +42 to +68
val interactionSource = remember { MutableInteractionSource() }
val isHovered by interactionSource.collectIsHoveredAsState()
val isFocused by interactionSource.collectIsFocusedAsState()

val borderColor = when {
isFocused -> colors.primary
isHovered -> colors.primary.copy(alpha = 0.6f)
else -> colors.border
}
val borderWidth = if (isFocused) Gort.borders.heavy else Gort.borders.thick

BasicTextField(
value = value,
onValueChange = onValueChange,
modifier = modifier
.fillMaxWidth()
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Text)
.clip(shape)
.background(colors.surface, shape)
.border(Gort.borders.thick, colors.border, shape)
.border(borderWidth, borderColor, shape)
.padding(horizontal = Gort.spacing.md, vertical = Gort.spacing.sm),
enabled = enabled,
singleLine = singleLine,
textStyle = textStyle,
cursorBrush = SolidColor(colors.primary),
decorationBox = { innerTextField ->
Box {
if (value.isEmpty()) {
BasicText(
text = placeholder,
style = textStyle.copy(color = colors.onSurface.copy(alpha = 0.4f)),
)
}
innerTextField()
}
},
)
}

@Composable
fun TextArea(
value: String,
onValueChange: (String) -> Unit,
modifier: Modifier = Modifier,
placeholder: String = "",
enabled: Boolean = true,
minLines: Int = 3,
shape: Shape = Gort.corners.default,
textStyle: TextStyle = Gort.typography.bodyMedium.copy(
fontFamily = FontFamily.Monospace,
color = Gort.colors.onSurface,
),
) {
val colors = Gort.colors

BasicTextField(
value = value,
onValueChange = onValueChange,
modifier = modifier
.fillMaxWidth()
.clip(shape)
.background(colors.surface, shape)
.border(Gort.borders.thick, colors.border, shape)
.padding(Gort.spacing.md),
enabled = enabled,
singleLine = false,
minLines = minLines,
textStyle = textStyle,
cursorBrush = SolidColor(colors.primary),
interactionSource = interactionSource,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd 'InteractionModifiers\.kt$' -t f -x sed -n '1,220p' {}
rg -n -C3 'gortTextInput\s*\(' --type=kt

Repository: ClankerGuru/gort

Length of output: 1733


🏁 Script executed:

fd 'TextArea\.kt$' -t f

Repository: ClankerGuru/gort

Length of output: 127


🏁 Script executed:

cat -n gort/src/commonMain/kotlin/zone/clanker/gort/components/TextArea.kt | head -100

Repository: ClankerGuru/gort

Length of output: 3667


Replace .hoverable() and .pointerHoverIcon() with gortTextInput.

Both TextField and TextArea duplicate the cursor and hover modifier setup. Replace lines 48–49 with:

.gortTextInput(interactionSource)

This keeps cursor behavior consistent across both components. The state collection (collectIsFocusedAsState(), collectIsHoveredAsState()) and border styling logic remain component-specific and should not be extracted.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/TextField.kt` around
lines 42 - 68, Replace the duplicated hover/cursor modifiers in TextField by
removing the .hoverable(interactionSource) and
.pointerHoverIcon(PointerIcon.Text) calls and instead apply the shared extension
.gortTextInput(interactionSource) on the modifier; keep the existing
interactionSource, collectIsFocusedAsState/collectIsHoveredAsState usage and
border styling in TextField (and do the same refactor in TextArea) so
cursor/hover behavior is centralized in gortTextInput while component-specific
state and styling remain unchanged.

Comment on lines +46 to +59
val borderColor = when {
isFocused -> colors.primary
isHovered -> colors.primary.copy(alpha = 0.6f)
else -> colors.border
}
val borderWidth = if (isFocused) Gort.borders.heavy else Gort.borders.thick

BasicTextField(
value = value,
onValueChange = onValueChange,
modifier = modifier
.fillMaxWidth()
.hoverable(interactionSource)
.pointerHoverIcon(PointerIcon.Text)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n gort/src/commonMain/kotlin/zone/clanker/gort/components/TextField.kt

Repository: ClankerGuru/gort

Length of output: 3656


🏁 Script executed:

# Check if hoverable has an enabled parameter in Compose Foundation
rg "hoverable" --type kotlin -B 2 -A 2 | head -50

Repository: ClankerGuru/gort

Length of output: 4996


🏁 Script executed:

# Search for pointerHoverIcon usage
rg "pointerHoverIcon" --type kotlin -B 2 -A 2 | head -50

Repository: ClankerGuru/gort

Length of output: 4961


🏁 Script executed:

# Check the full TextField implementation to understand the enabled flow
fd "TextField.kt" --type f -x cat -n {} \;

Repository: ClankerGuru/gort

Length of output: 3656


🏁 Script executed:

# Search for hoverable usage with parameters in the codebase
rg "hoverable\(" -B 1 -A 3 --type kotlin | head -60

Repository: ClankerGuru/gort

Length of output: 6044


🏁 Script executed:

# Check Compose version or build configuration
fd "build.gradle" -o "build.gradle.kts" | xargs cat | grep -i compose

Repository: ClankerGuru/gort

Length of output: 211


🌐 Web query:

Compose Foundation hoverable enabled parameter API

💡 Result:

The hoverable modifier in Jetpack Compose Foundation configures a composable to respond to hover events via pointer enter/exit. Signature: kotlin fun Modifier.hoverable( interactionSource: MutableInteractionSource, enabled: Boolean = true ): Modifier Parameters: - interactionSource: MutableInteractionSource that emits HoverInteraction.Enter when hovered. - enabled: Boolean (default true) that controls whether hover events are processed. When false, hover events are ignored. Usage example: kotlin @Composable fun HoverableSample { val interactionSource = remember { MutableInteractionSource } val isHovered by interactionSource.collectIsHoveredAsState Box( modifier = Modifier .size(128.dp) .background(if (isHovered) Color.Red else Color.Blue) .hoverable(interactionSource), contentAlignment = Alignment.Center ) { Text(if (isHovered) "Hovered" else "Unhovered") } } This API is part of androidx.compose.foundation:foundation (recent versions like 1.8.0-rc03+ as of 2025). Found in official Compose Foundation docs and API references.

Citations:


🏁 Script executed:

cat -n gort/src/commonMain/kotlin/zone/clanker/gort/components/TextArea.kt | head -80

Repository: ClankerGuru/gort

Length of output: 3667


🏁 Script executed:

# Check if any enabled state guards on hoverable exist anywhere
rg "hoverable.*enabled" --type kotlin

Repository: ClankerGuru/gort

Length of output: 42


Gate hover affordances behind enabled state.

When enabled = false, BasicTextField blocks editing but hoverable(interactionSource) still emits hover events, causing the border to highlight and pointerHoverIcon(PointerIcon.Text) to show the text cursor on disabled fields. Apply the enabled parameter to hoverable() and conditionally show pointerHoverIcon() to match the pattern already used in Pagination and NumberStepper components.

Suggested patch
-    val borderColor = when {
-        isFocused -> colors.primary
-        isHovered -> colors.primary.copy(alpha = 0.6f)
+    val showFocus = enabled && isFocused
+    val showHover = enabled && isHovered
+    val borderColor = when {
+        showFocus -> colors.primary
+        showHover -> colors.primary.copy(alpha = 0.6f)
         else -> colors.border
     }
-    val borderWidth = if (isFocused) Gort.borders.heavy else Gort.borders.thick
+    val borderWidth = if (showFocus) Gort.borders.heavy else Gort.borders.thick
@@
-            .hoverable(interactionSource)
-            .pointerHoverIcon(PointerIcon.Text)
+            .hoverable(interactionSource, enabled = enabled)
+            .then(
+                if (enabled) Modifier.pointerHoverIcon(PointerIcon.Text) else Modifier
+            )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/TextField.kt` around
lines 46 - 59, The hover affordances are currently active even when the
TextField is disabled; update the BasicTextField modifiers so hoverable uses the
component's enabled flag and only add pointerHoverIcon(PointerIcon.Text) when
enabled is true. Specifically, in the BasicTextField modifier chain (in
TextField.kt) change hoverable(interactionSource) to hoverable(enabled =
enabled, interactionSource = interactionSource) and wrap or conditionally
include pointerHoverIcon(PointerIcon.Text) so it is only applied when enabled is
true, matching the approach used in Pagination and NumberStepper.

Comment thread README.md
**Bold. Loud. Unapologetic.**

Gort is an opinionated UI component library that replaces Material 3 with thick borders, hard offset shadows, flat colors, and monospace type. Built on Compose Foundation — no Material dependency. Works on Android, iOS, Desktop, and Web.
Gort is an opinionated UI component library that replaces Material 3 with thick borders, hard offset shadows, flat colors, and monospace type. **52 components.** Pure Compose Foundation — zero Material dependency. Every platform Compose supports.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Component count mismatch: claims 52, but 57 listed.

The documentation states "52 components." but counting the component tables yields 57 components (Core Input: 15, Display: 12, Feedback: 8, Navigation: 6, Data & Compound: 11, Chat: 2, Foundation: 3).

Please reconcile the count with the actual component list.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 9, The README's component count string "**52
components.**" is incorrect; reconcile it with the actual component list (Core
Input, Display, Feedback, Navigation, Data & Compound, Chat, Foundation) and
update the count to the correct number (57) or adjust the listed components to
match 52; locate the sentence containing "52 components." in README.md and
replace it with the accurate count and/or update the category tables so the
total matches the stated number.

Comment thread README.md

---

## Components (52)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Section header also claims 52 components.

Same inconsistency as above — update to match the actual count.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 56, The section header "## Components (52)" is
inconsistent with the actual component list; update the header string in
README.md (the "## Components (52)" line) to reflect the true count of
components by replacing "(52)" with the actual number that matches the list
below so the header and the component list are consistent.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
catalog/build.gradle.kts (2)

5-10: Address deprecation warnings for future AGP compatibility.

The pipeline flags several deprecation warnings that will become breaking changes in Android Gradle Plugin 9.0.0+:

  1. androidTarget() is deprecated—the recommended approach is using com.android.kotlin.multiplatform.library plugin
  2. Combining org.jetbrains.kotlin.multiplatform with com.android.application in the same module will be unsupported

For a catalog/demo app this is acceptable short-term, but consider either:

  • Splitting the Android app into a separate subproject that depends on the KMP module
  • Migrating to the new com.android.kotlin.multiplatform.* plugin structure
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/build.gradle.kts` around lines 5 - 10, The build currently uses
kotlin { ... androidTarget() } in a module that also applies
org.jetbrains.kotlin.multiplatform alongside com.android.application, which
triggers deprecation warnings and will break with AGP 9+; either remove
androidTarget() and migrate this module to use the new
com.android.kotlin.multiplatform.library plugin (replace the kotlin
multiplatform Android target with the com.android.kotlin.multiplatform.* plugin
and adjust the kotlin { jvm("desktop") } configuration accordingly) or split the
Android app into a separate subproject that applies com.android.application and
depends on the KMP library module (keep org.jetbrains.kotlin.multiplatform in
the KMP module and remove any androidTarget() usage there); update plugin
declarations and module dependencies to reflect the chosen approach.

28-30: Move hardcoded activity-compose dependency version to version catalog.

The activity-compose version is hardcoded at line 29 while other dependencies use libs.versions.toml. For consistency and easier maintenance, move this to the version catalog. Note that version 1.10.1 is outdated; consider updating to 1.13.0 (latest stable) while refactoring.

♻️ Suggested change

In gradle/libs.versions.toml:

 [versions]
 kotlin = "2.3.0"
 compose-multiplatform = "1.10.0"
 agp = "8.7.3"
+activity-compose = "1.13.0"

 [libraries]
+activity-compose = { module = "androidx.activity:activity-compose", version.ref = "activity-compose" }

Then in catalog/build.gradle.kts:

 androidMain.dependencies {
-    implementation("androidx.activity:activity-compose:1.10.1")
+    implementation(libs.activity.compose)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/build.gradle.kts` around lines 28 - 30, Replace the hardcoded
dependency in the androidMain.dependencies block by moving the activity-compose
version into the version catalog and referencing it via libs; specifically add
an entry in libs.versions.toml (e.g., a version key and a library key like
androidx-activity-compose = { module = "androidx.activity:activity-compose",
version.ref = "activity-compose" } with activity-compose = "1.13.0"), then
update the implementation("androidx.activity:activity-compose:1.10.1") call in
the androidMain.dependencies to use
implementation(libs.androidx.activity.compose) (or the exact alias you chose) so
the version is managed from the catalog.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@catalog/build.gradle.kts`:
- Around line 5-10: The build currently uses kotlin { ... androidTarget() } in a
module that also applies org.jetbrains.kotlin.multiplatform alongside
com.android.application, which triggers deprecation warnings and will break with
AGP 9+; either remove androidTarget() and migrate this module to use the new
com.android.kotlin.multiplatform.library plugin (replace the kotlin
multiplatform Android target with the com.android.kotlin.multiplatform.* plugin
and adjust the kotlin { jvm("desktop") } configuration accordingly) or split the
Android app into a separate subproject that applies com.android.application and
depends on the KMP library module (keep org.jetbrains.kotlin.multiplatform in
the KMP module and remove any androidTarget() usage there); update plugin
declarations and module dependencies to reflect the chosen approach.
- Around line 28-30: Replace the hardcoded dependency in the
androidMain.dependencies block by moving the activity-compose version into the
version catalog and referencing it via libs; specifically add an entry in
libs.versions.toml (e.g., a version key and a library key like
androidx-activity-compose = { module = "androidx.activity:activity-compose",
version.ref = "activity-compose" } with activity-compose = "1.13.0"), then
update the implementation("androidx.activity:activity-compose:1.10.1") call in
the androidMain.dependencies to use
implementation(libs.androidx.activity.compose) (or the exact alias you chose) so
the version is managed from the catalog.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 959d2d9c-6d82-4e3e-b98b-518629a8da10

📥 Commits

Reviewing files that changed from the base of the PR and between eccfdea and ced324c.

📒 Files selected for processing (3)
  • catalog/build.gradle.kts
  • catalog/src/androidMain/AndroidManifest.xml
  • catalog/src/androidMain/kotlin/zone/clanker/gort/catalog/MainActivity.kt

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (9)
gort/src/commonMain/kotlin/zone/clanker/gort/components/Card.kt (1)

25-27: Handle negative shadow offsets to avoid start/top clipping.

Lines 25–27 only protect right/bottom overflow. If a caller passes negative offsets, shadow can still clip on start/top.

💡 Proposed refactor
     Surface(
-        modifier = modifier.padding(
-            end = shadow.offsetX.coerceAtLeast(0.dp),
-            bottom = shadow.offsetY.coerceAtLeast(0.dp),
-        ),
+        modifier = modifier.padding(
+            start = (-shadow.offsetX).coerceAtLeast(0.dp),
+            top = (-shadow.offsetY).coerceAtLeast(0.dp),
+            end = shadow.offsetX.coerceAtLeast(0.dp),
+            bottom = shadow.offsetY.coerceAtLeast(0.dp),
+        ),
         color = color,
         shadow = shadow,
         shape = shape,
     ) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Card.kt` around lines
25 - 27, The current padding only protects end/bottom using
shadow.offsetX.coerceAtLeast(0.dp) and shadow.offsetY.coerceAtLeast(0.dp), so
negative shadow.offsetX/offsetY still allow start/top clipping; update the
padding call in Card.kt to also add start =
(-shadow.offsetX).coerceAtLeast(0.dp) and top =
(-shadow.offsetY).coerceAtLeast(0.dp) (or equivalent using coerceAtMost and
abs), so negative offsets translate into positive start/top padding while
keeping existing end/bottom logic.
opsx/changes/catalog-overhaul/proposal.md (1)

225-225: Use hyphenated compound adjective for consistency.

Consider changing “full screen” to “full-screen” in this context.

Proposed tweak
-4. **Snackbar shadow** → Constrain shadow to snackbar width, not full screen
+4. **Snackbar shadow** → Constrain shadow to snackbar width, not full-screen
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@opsx/changes/catalog-overhaul/proposal.md` at line 225, Change the compound
adjective "full screen" to hyphenated "full-screen" in the proposal heading
"Snackbar shadow → Constrain shadow to snackbar width, not full screen" so it
reads "Snackbar shadow → Constrain shadow to snackbar width, not full-screen";
update any identical occurrences in that section to maintain consistent
hyphenation.
gort/src/commonMain/kotlin/zone/clanker/gort/foundation/GortIndication.kt (1)

20-28: Consider using System.identityHashCode(this) for hashCode.

Returning -1 works for a singleton but is unconventional. If multiple indication factories were ever stored in a hash-based collection, this would cause collisions.

♻️ Suggested improvement
-    override fun hashCode(): Int = -1
+    override fun hashCode(): Int = System.identityHashCode(this)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/foundation/GortIndication.kt`
around lines 20 - 28, The singleton object GortIndication currently returns a
constant -1 from hashCode which can cause collisions; change its hashCode
implementation to return System.identityHashCode(this) so it uses the object's
identity while keeping equals as reference equality; update the override fun
hashCode(): Int in GortIndication (the object that implements
IndicationNodeFactory and defines create(...) returning GortIndicationNode) to
return System.identityHashCode(this).
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt (2)

1-10: Add missing Color import to avoid fully-qualified reference.

Line 31 uses androidx.compose.ui.graphics.Color as a fully-qualified type. Since the file already imports from androidx.compose.ui, add the Color import for consistency.

🧹 Suggested fix
 import androidx.compose.foundation.layout.*
 import androidx.compose.foundation.text.BasicText
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.unit.dp
 import zone.clanker.gort.theme.Gort
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt`
around lines 1 - 10, Add an import for Color to avoid the fully-qualified
reference: update ThemeScreen.kt imports to include
androidx.compose.ui.graphics.Color so the code can use Color instead of
androidx.compose.ui.graphics.Color (this affects the type usage in the
composable that currently references the fully-qualified Color on the line where
Color is used, e.g., in the ThemeScreen composable).

30-47: Consider extracting nested composable to file scope.

Nested @Composable functions can cause unnecessary recompositions since they're redefined on each call. For a demo screen this is acceptable, but extracting ColorSwatch to file scope (as a private function) would be cleaner.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt`
around lines 30 - 47, ColorSwatch is currently defined as a nested `@Composable`
which causes it to be redefined on every call; move it out to file scope as a
private top-level `@Composable` function named ColorSwatch (keeping the same
signature: label: String, color: Color) and update any callers to use the new
top-level function; ensure you import androidx.compose.runtime.Composable if
needed and keep the Modifier, Box, and BasicText implementation identical so
behavior stays the same.
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt (2)

116-120: Consider adding accessibility semantics to the dark mode toggle.

The emoji toggle uses clickable {} without role or content description. Screen readers won't identify this as an interactive toggle. Consider using Modifier.semantics { role = Role.Button; contentDescription = "..." } or toggleable() for better accessibility.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt` around
lines 116 - 120, The dark-mode toggle implemented with BasicText +
Modifier.clickable (see BasicText and the Modifier.clickable(onClick =
onToggleDark) usage) lacks accessibility semantics; update the modifier to
include semantics with a Role.Button and a meaningful contentDescription (e.g.,
"Toggle dark mode") or replace clickable with Modifier.toggleable(checked =
isDark, onValueChange = onToggleDark) and add Modifier.semantics {
contentDescription = "Toggle dark mode" } so screen readers recognize and
announce the control correctly. Ensure the same content description reflects the
current state if desired (e.g., "Enable dark mode" / "Disable dark mode").

176-214: Nine items in bottom navigation may be cramped on compact screens.

With 9 CatalogSection entries and SpaceEvenly arrangement, the bottom nav could be quite dense on phones (< 600dp). Consider either:

  • Limiting bottom nav to top 4-5 sections with a "More" overflow
  • Using horizontal scrolling for the nav bar

For a demo/catalog app this is acceptable, but worth considering for better touch targets.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt` around
lines 176 - 214, BottomNavBar currently lays out all CatalogSection.entries with
Row and Arrangement.SpaceEvenly which will cram nine items on narrow screens;
change BottomNavBar to use a horizontally scrollable container (e.g., replace
the Row with a LazyRow or a Row +
Modifier.horizontalScroll(rememberScrollState())) so entries can scroll on small
widths, keep the existing click handling (onSelect) and selection logic
(currentSection/isSelected), and ensure each item (the Column rendering
section.icon and optional label via showLabels and
section.label.substringAfter(" ")) has a minimum touch target (e.g., increased
padding or minSize) so taps remain comfortable; alternatively, if you prefer
overflow, implement a "More" entry that groups less-used CatalogSection entries
behind a dropdown, but the simplest immediate fix is switching to a scrollable
row and enforcing min touch sizes.
gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortTypography.kt (1)

143-163: Significant duplication with data class defaults; consider DRY refactor.

The withFonts() method duplicates all 15 TextStyle definitions already present as defaults in the data class (lines 12-114). If any style attributes (fontSize, lineHeight, letterSpacing, fontWeight) change in the defaults, this method will drift out of sync.

Additionally, the method ignores the body and display parameters for title*, label*, and code styles (they're hardcoded to Monospace). This may be intentional for the neobrutalist aesthetic, but the KDoc at line 142 doesn't mention this behavior.

💡 Possible approach

Consider using GortTypography() defaults and calling .copy() on each field to only override fontFamily, reducing duplication:

fun withFonts(
    body: FontFamily,
    display: FontFamily = body,
): GortTypography {
    val defaults = GortTypography()
    return defaults.copy(
        displayLarge = defaults.displayLarge.copy(fontFamily = display),
        displayMedium = defaults.displayMedium.copy(fontFamily = display),
        // ... etc for headline/body styles
        // title/label/code left unchanged (Monospace)
    )
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortTypography.kt` around
lines 143 - 163, The withFonts function duplicates all GortTypography defaults;
change it to instantiate a defaults val = GortTypography() and return
defaults.copy(...) where you only .copy(fontFamily = display) or
.copy(fontFamily = body) for the relevant TextStyle fields (e.g., displayLarge,
displayMedium, displaySmall, headlineLarge/Medium/Small, bodyLarge/Medium/Small)
to avoid drifting from data-class defaults, leave title*, label*, and code as
Monospace if that is intentional, and update the KDoc for withFonts to document
that title/label/code intentionally remain Monospace.
gort/src/commonMain/kotlin/zone/clanker/gort/foundation/WindowSize.kt (1)

1-7: Unused import: BoxWithConstraints.

Line 3 imports BoxWithConstraints but only BoxWithConstraintsScope (line 4) is used in this file.

🧹 Suggested fix
 package zone.clanker.gort.foundation
 
-import androidx.compose.foundation.layout.BoxWithConstraints
 import androidx.compose.foundation.layout.BoxWithConstraintsScope
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.compositionLocalOf
 import androidx.compose.ui.unit.dp
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/foundation/WindowSize.kt` around
lines 1 - 7, Remove the unused import of BoxWithConstraints from WindowSize.kt:
keep the existing import of BoxWithConstraintsScope and the other imports
(Composable, compositionLocalOf, dp) but delete the line importing
BoxWithConstraints so only BoxWithConstraintsScope is imported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortColorScheme.kt`:
- Around line 139-145: The hue calculation can produce values less than -360 due
to the signed float remainder; compute the raw hue (h) as done now from
delta/cmax/r/g/b, then normalize it safely before constructing Hsl by applying a
two-step float-safe mod normalization such as ((h % 360f) + 360f) % 360f (or
equivalent) so any negative or out-of-range float is mapped into [0,360); update
the symbol h in the when block (or immediately after) and use the normalized
value in the Hsl(...) return.

In `@opsx/changes/catalog-overhaul/proposal.md`:
- Around line 29-48: The fenced code block containing the ASCII UI mock (the
block that begins with the box art line
"╔══════════════════════════════════════╗" and the "BUTTON" headline) is missing
a language tag (MD040); add a language identifier to the opening backtick fence
(for example ```text) so the block becomes a fenced code block with a language
tag, keep the existing closing ``` unchanged, and ensure the tag is applied to
that specific ASCII-art code block.

---

Nitpick comments:
In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt`:
- Around line 116-120: The dark-mode toggle implemented with BasicText +
Modifier.clickable (see BasicText and the Modifier.clickable(onClick =
onToggleDark) usage) lacks accessibility semantics; update the modifier to
include semantics with a Role.Button and a meaningful contentDescription (e.g.,
"Toggle dark mode") or replace clickable with Modifier.toggleable(checked =
isDark, onValueChange = onToggleDark) and add Modifier.semantics {
contentDescription = "Toggle dark mode" } so screen readers recognize and
announce the control correctly. Ensure the same content description reflects the
current state if desired (e.g., "Enable dark mode" / "Disable dark mode").
- Around line 176-214: BottomNavBar currently lays out all
CatalogSection.entries with Row and Arrangement.SpaceEvenly which will cram nine
items on narrow screens; change BottomNavBar to use a horizontally scrollable
container (e.g., replace the Row with a LazyRow or a Row +
Modifier.horizontalScroll(rememberScrollState())) so entries can scroll on small
widths, keep the existing click handling (onSelect) and selection logic
(currentSection/isSelected), and ensure each item (the Column rendering
section.icon and optional label via showLabels and
section.label.substringAfter(" ")) has a minimum touch target (e.g., increased
padding or minSize) so taps remain comfortable; alternatively, if you prefer
overflow, implement a "More" entry that groups less-used CatalogSection entries
behind a dropdown, but the simplest immediate fix is switching to a scrollable
row and enforcing min touch sizes.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt`:
- Around line 1-10: Add an import for Color to avoid the fully-qualified
reference: update ThemeScreen.kt imports to include
androidx.compose.ui.graphics.Color so the code can use Color instead of
androidx.compose.ui.graphics.Color (this affects the type usage in the
composable that currently references the fully-qualified Color on the line where
Color is used, e.g., in the ThemeScreen composable).
- Around line 30-47: ColorSwatch is currently defined as a nested `@Composable`
which causes it to be redefined on every call; move it out to file scope as a
private top-level `@Composable` function named ColorSwatch (keeping the same
signature: label: String, color: Color) and update any callers to use the new
top-level function; ensure you import androidx.compose.runtime.Composable if
needed and keep the Modifier, Box, and BasicText implementation identical so
behavior stays the same.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/Card.kt`:
- Around line 25-27: The current padding only protects end/bottom using
shadow.offsetX.coerceAtLeast(0.dp) and shadow.offsetY.coerceAtLeast(0.dp), so
negative shadow.offsetX/offsetY still allow start/top clipping; update the
padding call in Card.kt to also add start =
(-shadow.offsetX).coerceAtLeast(0.dp) and top =
(-shadow.offsetY).coerceAtLeast(0.dp) (or equivalent using coerceAtMost and
abs), so negative offsets translate into positive start/top padding while
keeping existing end/bottom logic.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/foundation/GortIndication.kt`:
- Around line 20-28: The singleton object GortIndication currently returns a
constant -1 from hashCode which can cause collisions; change its hashCode
implementation to return System.identityHashCode(this) so it uses the object's
identity while keeping equals as reference equality; update the override fun
hashCode(): Int in GortIndication (the object that implements
IndicationNodeFactory and defines create(...) returning GortIndicationNode) to
return System.identityHashCode(this).

In `@gort/src/commonMain/kotlin/zone/clanker/gort/foundation/WindowSize.kt`:
- Around line 1-7: Remove the unused import of BoxWithConstraints from
WindowSize.kt: keep the existing import of BoxWithConstraintsScope and the other
imports (Composable, compositionLocalOf, dp) but delete the line importing
BoxWithConstraints so only BoxWithConstraintsScope is imported.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortTypography.kt`:
- Around line 143-163: The withFonts function duplicates all GortTypography
defaults; change it to instantiate a defaults val = GortTypography() and return
defaults.copy(...) where you only .copy(fontFamily = display) or
.copy(fontFamily = body) for the relevant TextStyle fields (e.g., displayLarge,
displayMedium, displaySmall, headlineLarge/Medium/Small, bodyLarge/Medium/Small)
to avoid drifting from data-class defaults, leave title*, label*, and code as
Monospace if that is intentional, and update the KDoc for withFonts to document
that title/label/code intentionally remain Monospace.

In `@opsx/changes/catalog-overhaul/proposal.md`:
- Line 225: Change the compound adjective "full screen" to hyphenated
"full-screen" in the proposal heading "Snackbar shadow → Constrain shadow to
snackbar width, not full screen" so it reads "Snackbar shadow → Constrain shadow
to snackbar width, not full-screen"; update any identical occurrences in that
section to maintain consistent hyphenation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f3a6a44d-27d2-4964-b515-122b624ed890

📥 Commits

Reviewing files that changed from the base of the PR and between ced324c and 7505266.

📒 Files selected for processing (12)
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Card.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Marquee.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/SegmentedControl.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Snackbar.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/foundation/GortIndication.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/foundation/WindowSize.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortColorScheme.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortTypography.kt
  • opsx/changes/catalog-overhaul/proposal.md
✅ Files skipped from review due to trivial changes (1)
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Snackbar.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/SegmentedControl.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/foundation/InteractionModifiers.kt

Comment on lines +139 to +145
val h = when {
delta == 0f -> 0f
cmax == r -> 60f * (((g - b) / delta) % 6f)
cmax == g -> 60f * (((b - r) / delta) + 2f)
else -> 60f * (((r - g) / delta) + 4f)
}
return Hsl((h + 360f) % 360f, s.coerceIn(0f, 1f), l.coerceIn(0f, 1f))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential negative hue from modulo operation.

At line 141, when cmax == r and g < b, the expression ((g - b) / delta) % 6f can yield a negative result (Kotlin's % preserves the sign of the dividend). While line 145 normalizes with (h + 360f) % 360f, if the intermediate h is between -360 and 0, this works correctly. However, if h is less than -360 (theoretically possible with extreme values), one addition of 360 may not suffice.

Consider using floorMod or a safe normalization:

🛡️ Safer normalization
-        return Hsl((h + 360f) % 360f, s.coerceIn(0f, 1f), l.coerceIn(0f, 1f))
+        return Hsl(((h % 360f) + 360f) % 360f, s.coerceIn(0f, 1f), l.coerceIn(0f, 1f))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/theme/GortColorScheme.kt` around
lines 139 - 145, The hue calculation can produce values less than -360 due to
the signed float remainder; compute the raw hue (h) as done now from
delta/cmax/r/g/b, then normalize it safely before constructing Hsl by applying a
two-step float-safe mod normalization such as ((h % 360f) + 360f) % 360f (or
equivalent) so any negative or out-of-range float is mapped into [0,360); update
the symbol h in the when block (or immediately after) and use the normalized
value in the Hsl(...) return.

Comment on lines +29 to +48
```
╔══════════════════════════════════════╗
║ BUTTON [⚙️] ║ ← headline + settings toggle
╠══════════════════════════════════════╣
║ The workhorse of user interaction. ║ ← description/lede
║ Neobrutalist buttons use hard ║
║ shadows that collapse on press. ║
╠──────────────────────────────────────╣
║ ║
║ ┌─────────────────┐ ║ ← live demo area
║ │ Primary Button │░░ ║ (cream inset background)
║ └─────────────────┘ ║
║ ║
╠──────────────────────────────────────╣
║ Variant: [Primary|Secondary|Outline] ║ ← interactive controls
║ Enabled: [✓] Size: [M] ║ (toggle variants live)
╠──────────────────────────────────────╣
║ ▸ View Code ║ ← expandable code snippet
╚══════════════════════════════════════╝
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a language tag to the fenced block.

The code fence starting at Line 29 has no language identifier (MD040).

Proposed fix
-```
+```text
╔══════════════════════════════════════╗
║ BUTTON                         [⚙️] ║  ← headline + settings toggle
╠══════════════════════════════════════╣
...
╚══════════════════════════════════════╝
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```
╔══════════════════════════════════════╗
║ BUTTON [⚙️] ║ ← headline + settings toggle
╠══════════════════════════════════════╣
║ The workhorse of user interaction. ║ ← description/lede
║ Neobrutalist buttons use hard ║
║ shadows that collapse on press. ║
╠──────────────────────────────────────╣
║ ║
║ ┌─────────────────┐ ║ ← live demo area
║ │ Primary Button │░░ ║ (cream inset background)
║ └─────────────────┘ ║
║ ║
╠──────────────────────────────────────╣
║ Variant: [Primary|Secondary|Outline] ║ ← interactive controls
║ Enabled: [] Size: [M] ║ (toggle variants live)
╠──────────────────────────────────────╣
║ ▸ View Code ║ ← expandable code snippet
╚══════════════════════════════════════╝
```
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 29-29: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@opsx/changes/catalog-overhaul/proposal.md` around lines 29 - 48, The fenced
code block containing the ASCII UI mock (the block that begins with the box art
line "╔══════════════════════════════════════╗" and the "BUTTON" headline) is
missing a language tag (MD040); add a language identifier to the opening
backtick fence (for example ```text) so the block becomes a fenced code block
with a language tag, keep the existing closing ``` unchanged, and ensure the tag
is applied to that specific ASCII-art code block.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (9)
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CompoundScreen.kt (1)

54-65: Code snippet omits "Groovy" from items list.

The displayed code snippet shows items = listOf("Kotlin", "Java", "Scala"), but the actual demo uses listOf("Kotlin", "Java", "Scala", "Groovy"). Consider aligning them for documentation accuracy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CompoundScreen.kt`
around lines 54 - 65, The code example for GortDropdown is missing "Groovy" in
the items list; update the snippet so the displayed items match the demo by
changing the items list in the GortDropdown example to include "Groovy" (i.e.,
items = listOf("Kotlin", "Java", "Scala", "Groovy")) and ensure the example uses
the same selectedItem/ onItemSelected pattern (dropdownValue) as in the demo.
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ChatScreen.kt (1)

17-38: Minor mismatch between code snippet and live demo.

The code snippet shows "Looks incredible!" (Line 23), but the actual rendered ChatBubble shows "Yes! 50 components, all neobrutalist. Looks incredible." (Line 35). This is a minor documentation inconsistency—consider aligning them for clarity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ChatScreen.kt` around
lines 17 - 38, The code snippet's first ChatBubble message ("Looks incredible!")
doesn't match the rendered second ChatBubble message ("Yes! 50 components, all
neobrutalist. Looks incredible."); update the snippet so the message parameter
in the first ChatBubble inside the triple-quoted `code = """..."""` block
matches the rendered `ChatBubble(message = "Yes! 50 components, all
neobrutalist. Looks incredible.")`, or alternatively change the rendered inline
`ChatBubble` call to the shorter string—ensure the `ChatBubble` `message` values
are consistent between the `code =` example and the live `ChatBubble(...)`
invocation.
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt (2)

78-90: Code snippet shows 3 options, but demo renders 4.

The code example shows options = listOf("Day", "Week", "Month"), while the actual demo includes "Year" as a fourth option. Consider aligning for documentation accuracy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt`
around lines 78 - 90, The code sample in ButtonsScreen.kt shows SegmentedControl
with options = listOf("Day", "Week", "Month") but the demo renders four options;
update the options list in the code example to match the rendered demo by adding
"Year" (i.e., change options = listOf("Day", "Week", "Month") to options =
listOf("Day", "Week", "Month", "Year")) or alternatively remove "Year" from the
demo; make the change where SegmentedControl and the segmentIndex state are
defined so the example and rendered control match.

23-30: Fragile coupling between SegmentedControl index and ButtonVariant enum ordinal.

The mapping ButtonVariant.entries[it] assumes the enum declaration order matches the UI options list. If ButtonVariant is reordered, this will silently break. Consider an explicit mapping for robustness.

♻️ Use explicit mapping instead of index
+            val variants = listOf(
+                ButtonVariant.Primary,
+                ButtonVariant.Secondary,
+                ButtonVariant.Outline,
+                ButtonVariant.Danger,
+            )
             SegmentedControl(
                 options = listOf("Primary", "Secondary", "Outline", "Danger"),
                 selectedIndex = segIdx,
                 onSelect = {
                     segIdx = it
-                    variant = ButtonVariant.entries[it]
+                    variant = variants[it]
                 },
             )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt`
around lines 23 - 30, The code currently couples SegmentedControl's selected
index (segIdx) to ButtonVariant by using ButtonVariant.entries[it], which will
break if the enum order changes; replace this fragile ordinal mapping with an
explicit mapping list (e.g., a val like variantsForOptions =
listOf(ButtonVariant.Primary, ButtonVariant.Secondary, ButtonVariant.Outline,
ButtonVariant.Danger)) and use that list to set variant (in the SegmentedControl
onSelect and any initial state) so the UI option labels in SegmentedControl and
the ButtonVariant values remain explicitly paired regardless of enum reordering.
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ComponentShowcase.kt (1)

77-81: Consider adding accessibility semantics to the clickable text.

The "View Code" / "Hide Code" toggle uses a bare clickable modifier without role or content description, which may not be ideal for screen readers. Consider using Modifier.clickable(role = Role.Button) or wrapping in an accessible component.

♻️ Optional accessibility improvement
+import androidx.compose.ui.semantics.Role
 ...
                 BasicText(
                     text = if (expanded) "▾ Hide Code" else "▸ View Code",
                     style = Gort.typography.labelMedium.copy(color = colors.primary),
-                    modifier = Modifier.clickable { expanded = !expanded }.padding(vertical = 4.dp),
+                    modifier = Modifier
+                        .clickable(role = Role.Button) { expanded = !expanded }
+                        .padding(vertical = 4.dp),
                 )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ComponentShowcase.kt`
around lines 77 - 81, The clickable BasicText toggle uses a bare
Modifier.clickable; update the modifier on the BasicText in ComponentShowcase to
expose accessibility semantics by using Modifier.clickable(role = Role.Button)
and add a semantic content description (e.g., via Modifier.semantics {
contentDescription = if (expanded) "Hide code" else "View code" }) so screen
readers announce it as a button and provide the correct label for the expanded
state; reference the BasicText call, the expanded state variable, and the
Modifier.clickable usage when making the change.
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/InputsScreen.kt (2)

33-44: Two independent Checkbox instances with separate state.

The controls slot (Line 38-39) and content slot (Line 42-43) each declare their own checked state, resulting in two independent checkboxes. If the intent is for controls to drive the demo, they should share state. If both are intentional demos, consider clarifying.

♻️ Option: Share state between controls and content
     ComponentShowcase(
         name = "Checkbox",
         description = "Boolean toggle for options that can be independently enabled or disabled.",
         code = """Checkbox(checked = checked, onCheckedChange = { checked = it })""",
-        controls = {
-            var checked by remember { mutableStateOf(false) }
-            Checkbox(checked = checked, onCheckedChange = { checked = it })
-        },
     ) {
-        var checked by remember { mutableStateOf(true) }
+        var checked by remember { mutableStateOf(false) }
         Checkbox(checked = checked, onCheckedChange = { checked = it })
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/InputsScreen.kt`
around lines 33 - 44, The controls and content blocks inside ComponentShowcase
each create their own `checked` state so the two Checkbox instances are
independent; to fix, lift the `var checked by remember { mutableStateOf(...) }`
out of the inner `controls` and `content` lambdas into a single shared state
declared before calling `ComponentShowcase` (or pass a single `checked`
state/handler into both slots) so both the `Checkbox` in `controls` and the
`Checkbox` in the main content use the same `checked` variable; alternatively,
if two independent demos were intended, rename one state variable and add a
clarifying description in `ComponentShowcase` to make that explicit.

51-56: Radio buttons are rendered without visible labels.

The options list has labels ("Option A", "Option B", "Option C"), but they are ignored (_). Users see three unlabeled radio buttons, making it unclear what each represents.

♻️ Add visible labels to radio buttons
+import androidx.compose.foundation.text.BasicText
 ...
         var radio by remember { mutableIntStateOf(0) }
         Column(verticalArrangement = Arrangement.spacedBy(Gort.spacing.sm)) {
-            listOf("Option A", "Option B", "Option C").forEachIndexed { i, _ ->
-                RadioButton(selected = radio == i, onClick = { radio = i })
+            listOf("Option A", "Option B", "Option C").forEachIndexed { i, label ->
+                Row(horizontalArrangement = Arrangement.spacedBy(Gort.spacing.sm)) {
+                    RadioButton(selected = radio == i, onClick = { radio = i })
+                    BasicText(label, style = Gort.typography.body.copy(color = Gort.colors.onSurface))
+                }
             }
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/InputsScreen.kt`
around lines 51 - 56, The radio buttons render without visible labels because
the forEachIndexed ignores the string parameter; change the lambda from { i, _
-> } to { i, label -> } (referencing the listOf(...).forEachIndexed) and render
the label alongside each RadioButton (e.g., wrap RadioButton and Text(label) in
a Row and/or add Modifier.clickable { radio = i }) so users can see and click
the option; update usages of the 'radio' state and the RadioButton(onClick)
accordingly to set radio = i when either the button or row/text is clicked.
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt (2)

161-183: Navigation items lack accessibility semantics.

The clickable nav items don't expose their selected state to accessibility services. Consider adding selectable semantics so screen readers can announce "Theme, selected" vs "Buttons, not selected".

Modifier
    .selectable(
        selected = isSelected,
        onClick = { onSelect(section) },
        role = Role.Tab,
    )

Same applies to BottomNavBar (lines 204-209).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt` around
lines 161 - 183, The nav item Column currently uses Modifier.clickable and
doesn't expose selection semantics; replace the clickable modifier with
Modifier.selectable(selected = isSelected, onClick = { onSelect(section) }, role
= Role.Tab) so accessibility services can announce selected state for the Column
used in CatalogApp (the composable using Column, BasicText, and onSelect/
isSelected). Do the same change in BottomNavBar where nav items are built (use
selectable with the appropriate selected boolean and onClick) so screen readers
report "selected"/"not selected".

23-33: Consider eliminating redundant icon data.

The icon is stored twice: once in icon and once as a prefix in label. The substringAfter(" ") extraction pattern (lines 178, 216) relies on all labels following the "emoji space text" convention.

A cleaner approach would be to store only the text portion in label and compose the display string when needed:

♻️ Suggested refactor
-enum class CatalogSection(val label: String, val icon: String) {
-    Theme("🎨 Theme", "🎨"),
-    Buttons("🔘 Actions", "🔘"),
+enum class CatalogSection(val label: String, val icon: String) {
+    Theme("Theme", "🎨"),
+    Buttons("Actions", "🔘"),
     // ... etc

Then use section.label directly instead of section.label.substringAfter(" ").

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt` around
lines 23 - 33, The enum CatalogSection currently duplicates the emoji in both
icon and the label prefix; update the enum entries in CatalogSection so label
contains only the text (e.g., Theme, Buttons, Inputs, etc.) and keep the emoji
in icon, then replace every use of section.label.substringAfter(" ") (the
occurrences in the rendering code that build display strings) with a composed
string using the icon and label (e.g., "${section.icon} ${section.label}") so
the UI composes the emoji + text at render time and no longer relies on
substringAfter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt`:
- Around line 50-53: The toggle state can drift because isDark and currentColors
are updated independently; pick one source of truth—prefer deriving isDark from
currentColors: remove the separate mutable isDark state and compute isDark from
currentColors (or use a derivedStateOf) so Masthead(isDark=...) always reflects
currentColors; change the onToggleDark handler to flip currentColors between
GortColors.dark() and GortColors.light() (instead of toggling isDark), and
ensure ThemeScreen’s onColorsChange only mutates currentColors (no separate
isDark mutation). Alternatively, if you keep isDark, have ThemeScreen expose
dark mode changes (e.g., onDarkModeChange) and update both isDark and
currentColors together in that callback so they never diverge.
- Around line 128-132: The dark-mode toggle BasicText currently uses
Modifier.clickable(onClick = onToggleDark) but lacks accessibility semantics;
update the Modifier for the BasicText (or replace with an
IconButton/ClickableText) to include semantics: set role = Role.Button and a
meaningful contentDescription based on isDark (e.g., "Switch to light mode" when
isDark is true, otherwise "Switch to dark mode"), preserving the existing
onToggleDark callback and the existing style (Gort.typography.titleLarge) so
screen readers will announce it as a button with purpose.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/DisplayScreen.kt`:
- Around line 144-162: The SegmentedControl's state (typeIdx) is unused—either
remove the control or wire it to render only the selected Callout; to fix, keep
the existing mutableIntStateOf typeIdx and replace the three static Callout
blocks with a single when(typeIdx) branch that renders one Callout using
Callout(type = CalloutType.Info/Warning/Danger, title = ...) and the matching
BasicText for each case so the SegmentedControl (onSelect) controls which
Callout is shown; ensure you reference the existing symbols typeIdx,
SegmentedControl, Callout, and CalloutType when implementing the conditional
rendering.

---

Nitpick comments:
In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt`:
- Around line 78-90: The code sample in ButtonsScreen.kt shows SegmentedControl
with options = listOf("Day", "Week", "Month") but the demo renders four options;
update the options list in the code example to match the rendered demo by adding
"Year" (i.e., change options = listOf("Day", "Week", "Month") to options =
listOf("Day", "Week", "Month", "Year")) or alternatively remove "Year" from the
demo; make the change where SegmentedControl and the segmentIndex state are
defined so the example and rendered control match.
- Around line 23-30: The code currently couples SegmentedControl's selected
index (segIdx) to ButtonVariant by using ButtonVariant.entries[it], which will
break if the enum order changes; replace this fragile ordinal mapping with an
explicit mapping list (e.g., a val like variantsForOptions =
listOf(ButtonVariant.Primary, ButtonVariant.Secondary, ButtonVariant.Outline,
ButtonVariant.Danger)) and use that list to set variant (in the SegmentedControl
onSelect and any initial state) so the UI option labels in SegmentedControl and
the ButtonVariant values remain explicitly paired regardless of enum reordering.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt`:
- Around line 161-183: The nav item Column currently uses Modifier.clickable and
doesn't expose selection semantics; replace the clickable modifier with
Modifier.selectable(selected = isSelected, onClick = { onSelect(section) }, role
= Role.Tab) so accessibility services can announce selected state for the Column
used in CatalogApp (the composable using Column, BasicText, and onSelect/
isSelected). Do the same change in BottomNavBar where nav items are built (use
selectable with the appropriate selected boolean and onClick) so screen readers
report "selected"/"not selected".
- Around line 23-33: The enum CatalogSection currently duplicates the emoji in
both icon and the label prefix; update the enum entries in CatalogSection so
label contains only the text (e.g., Theme, Buttons, Inputs, etc.) and keep the
emoji in icon, then replace every use of section.label.substringAfter(" ") (the
occurrences in the rendering code that build display strings) with a composed
string using the icon and label (e.g., "${section.icon} ${section.label}") so
the UI composes the emoji + text at render time and no longer relies on
substringAfter.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ChatScreen.kt`:
- Around line 17-38: The code snippet's first ChatBubble message ("Looks
incredible!") doesn't match the rendered second ChatBubble message ("Yes! 50
components, all neobrutalist. Looks incredible."); update the snippet so the
message parameter in the first ChatBubble inside the triple-quoted `code =
"""..."""` block matches the rendered `ChatBubble(message = "Yes! 50 components,
all neobrutalist. Looks incredible.")`, or alternatively change the rendered
inline `ChatBubble` call to the shorter string—ensure the `ChatBubble` `message`
values are consistent between the `code =` example and the live
`ChatBubble(...)` invocation.

In
`@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ComponentShowcase.kt`:
- Around line 77-81: The clickable BasicText toggle uses a bare
Modifier.clickable; update the modifier on the BasicText in ComponentShowcase to
expose accessibility semantics by using Modifier.clickable(role = Role.Button)
and add a semantic content description (e.g., via Modifier.semantics {
contentDescription = if (expanded) "Hide code" else "View code" }) so screen
readers announce it as a button and provide the correct label for the expanded
state; reference the BasicText call, the expanded state variable, and the
Modifier.clickable usage when making the change.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CompoundScreen.kt`:
- Around line 54-65: The code example for GortDropdown is missing "Groovy" in
the items list; update the snippet so the displayed items match the demo by
changing the items list in the GortDropdown example to include "Groovy" (i.e.,
items = listOf("Kotlin", "Java", "Scala", "Groovy")) and ensure the example uses
the same selectedItem/ onItemSelected pattern (dropdownValue) as in the demo.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/InputsScreen.kt`:
- Around line 33-44: The controls and content blocks inside ComponentShowcase
each create their own `checked` state so the two Checkbox instances are
independent; to fix, lift the `var checked by remember { mutableStateOf(...) }`
out of the inner `controls` and `content` lambdas into a single shared state
declared before calling `ComponentShowcase` (or pass a single `checked`
state/handler into both slots) so both the `Checkbox` in `controls` and the
`Checkbox` in the main content use the same `checked` variable; alternatively,
if two independent demos were intended, rename one state variable and add a
clarifying description in `ComponentShowcase` to make that explicit.
- Around line 51-56: The radio buttons render without visible labels because the
forEachIndexed ignores the string parameter; change the lambda from { i, _ -> }
to { i, label -> } (referencing the listOf(...).forEachIndexed) and render the
label alongside each RadioButton (e.g., wrap RadioButton and Text(label) in a
Row and/or add Modifier.clickable { radio = i }) so users can see and click the
option; update usages of the 'radio' state and the RadioButton(onClick)
accordingly to set radio = i when either the button or row/text is clicked.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aec93645-d3cf-46f5-8c3b-0a0806693811

📥 Commits

Reviewing files that changed from the base of the PR and between 7505266 and 0be009f.

📒 Files selected for processing (11)
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ChatScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ComponentShowcase.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CompoundScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/DataScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/DisplayScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/FeedbackScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/InputsScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/NavigationScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt
✅ Files skipped from review due to trivial changes (1)
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/NavigationScreen.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ThemeScreen.kt

Comment on lines +50 to +53
Masthead(isDark = isDark, onToggleDark = {
isDark = !isDark
currentColors = if (isDark) GortColors.dark() else GortColors.light()
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential state desync between isDark and currentColors.

When the user toggles dark mode here, both isDark and currentColors are updated together. However, if ThemeScreen calls onColorsChange with custom colors (e.g., from the theme playground), the isDark boolean won't update, causing the toggle icon to potentially show the wrong state.

Consider either:

  1. Deriving isDark from currentColors rather than tracking it separately, or
  2. Having ThemeScreen also expose dark mode changes so both states stay in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt` around
lines 50 - 53, The toggle state can drift because isDark and currentColors are
updated independently; pick one source of truth—prefer deriving isDark from
currentColors: remove the separate mutable isDark state and compute isDark from
currentColors (or use a derivedStateOf) so Masthead(isDark=...) always reflects
currentColors; change the onToggleDark handler to flip currentColors between
GortColors.dark() and GortColors.light() (instead of toggling isDark), and
ensure ThemeScreen’s onColorsChange only mutates currentColors (no separate
isDark mutation). Alternatively, if you keep isDark, have ThemeScreen expose
dark mode changes (e.g., onDarkModeChange) and update both isDark and
currentColors together in that callback so they never diverge.

Comment thread catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt Outdated
Comment on lines +144 to 162
controls = {
var typeIdx by remember { mutableIntStateOf(0) }
SegmentedControl(
options = listOf("Info", "Warning", "Danger"),
selectedIndex = typeIdx,
onSelect = { typeIdx = it },
)
},
) {
Callout(type = CalloutType.Info, title = "Note") {
BasicText("This is an informational callout.", style = Gort.typography.body.copy(color = Gort.colors.onSurface))
}
Callout(type = CalloutType.Warning, title = "Warning") {
BasicText("Be careful with this operation.", style = Gort.typography.body.copy(color = Gort.colors.onSurface))
}
Callout(type = CalloutType.Danger, title = "Danger") {
BasicText("This action cannot be undone.", style = Gort.typography.body.copy(color = Gort.colors.onSurface))
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

The typeIdx state is unused—the SegmentedControl doesn't control the displayed Callout.

The controls slot declares typeIdx and renders a SegmentedControl, but the content lambda ignores this state and renders all three Callout variants statically. Either remove the non-functional controls, or wire typeIdx to conditionally render the selected Callout type.

🔧 Option: Wire the SegmentedControl to control displayed Callout
     ComponentShowcase(
         name = "Callout",
         description = "Highlighted information block for tips, warnings, or danger notices.",
         code = """Callout(type = CalloutType.Info, title = "Note") {
     BasicText("Informational callout.")
 }""",
-        controls = {
-            var typeIdx by remember { mutableIntStateOf(0) }
-            SegmentedControl(
-                options = listOf("Info", "Warning", "Danger"),
-                selectedIndex = typeIdx,
-                onSelect = { typeIdx = it },
-            )
-        },
     ) {
-        Callout(type = CalloutType.Info, title = "Note") {
-            BasicText("This is an informational callout.", style = Gort.typography.body.copy(color = Gort.colors.onSurface))
-        }
-        Callout(type = CalloutType.Warning, title = "Warning") {
-            BasicText("Be careful with this operation.", style = Gort.typography.body.copy(color = Gort.colors.onSurface))
-        }
-        Callout(type = CalloutType.Danger, title = "Danger") {
-            BasicText("This action cannot be undone.", style = Gort.typography.body.copy(color = Gort.colors.onSurface))
+        var typeIdx by remember { mutableIntStateOf(0) }
+        SegmentedControl(
+            options = listOf("Info", "Warning", "Danger"),
+            selectedIndex = typeIdx,
+            onSelect = { typeIdx = it },
+        )
+        val type = listOf(CalloutType.Info, CalloutType.Warning, CalloutType.Danger)[typeIdx]
+        val (title, message) = when (type) {
+            CalloutType.Info -> "Note" to "This is an informational callout."
+            CalloutType.Warning -> "Warning" to "Be careful with this operation."
+            CalloutType.Danger -> "Danger" to "This action cannot be undone."
+        }
+        Callout(type = type, title = title) {
+            BasicText(message, style = Gort.typography.body.copy(color = Gort.colors.onSurface))
         }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/DisplayScreen.kt`
around lines 144 - 162, The SegmentedControl's state (typeIdx) is unused—either
remove the control or wire it to render only the selected Callout; to fix, keep
the existing mutableIntStateOf typeIdx and replace the three static Callout
blocks with a single when(typeIdx) branch that renders one Callout using
Callout(type = CalloutType.Info/Warning/Danger, title = ...) and the matching
BasicText for each case so the SegmentedControl (onSelect) controls which
Callout is shown; ensure you reference the existing symbols typeIdx,
SegmentedControl, Callout, and CalloutType when implementing the conditional
rendering.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt (1)

135-140: Accessibility improved but still missing semantic role.

The toggle now has contentDescription, which is good. However, the clickable modifier still lacks role = Role.Button, so screen readers won't announce it as an interactive button.

♿ Add semantic role
+import androidx.compose.ui.semantics.Role
 ...
             Image(
                 imageVector = if (isDark) Lucide.Sun else Lucide.Moon,
                 contentDescription = if (isDark) "Light mode" else "Dark mode",
                 colorFilter = ColorFilter.tint(colors.onSurface),
-                modifier = Modifier.size(22.dp).clickable(onClick = onToggleDark),
+                modifier = Modifier.size(22.dp).clickable(role = Role.Button, onClick = onToggleDark),
             )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt` around
lines 135 - 140, The Image used for the theme toggle (Image with imageVector and
modifier using Modifier.size(22.dp).clickable(onClick = onToggleDark)) is
missing an explicit semantic role; update the clickable modifier to include role
= Role.Button so assistive tech announces it as a button, e.g. modify the
Modifier.clickable call on the Image to pass the role parameter (keeping
onToggleDark as the onClick handler) and import/androidx/compose/ui/semantics
Role if needed.
gort/src/commonMain/kotlin/zone/clanker/gort/components/GortScaffold.kt (1)

25-28: Consider calculating actual PaddingValues from slot heights.

The content lambda receives an empty PaddingValues(), which differs from Material's Scaffold that provides computed padding based on topBar/bottomBar heights. While the current implementation works when callers ignore the padding (as CatalogApp does), it may surprise users expecting slot-aware padding.

Additionally, snackbarHost() is placed inside the same Box as content without explicit z-index ordering — ensure snackbars will appear above content as expected.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/GortScaffold.kt`
around lines 25 - 28, GortScaffold's content lambda is being called with an
empty PaddingValues() and snackbarHost() is placed in the same Box as content;
compute and pass real PaddingValues derived from the measured heights of the
topBar and bottomBar slots (measure topBar and bottomBar in the layout,
calculate inset padding (top = topBarHeight, bottom = bottomBarHeight) and pass
that into content(paddingValues)) and ensure snackbarHost() is rendered above
content by placing it in a separate Layer/Box with higher z-index or using
Modifier.zIndex on the snackbarHost composable; update the GortScaffold
implementation (look for the functions/slots named GortScaffold, content,
topBar, bottomBar, snackbarHost, and the Box containing content) to perform
these changes.
gort/src/commonMain/kotlin/zone/clanker/gort/components/GortDrawer.kt (1)

35-41: Add accessibility semantics to the scrim.

The scrim's clickable modifier lacks semantic role and description. Screen readers won't announce this as a dismissible overlay.

♿ Suggested improvement
+import androidx.compose.ui.semantics.onClick
+import androidx.compose.ui.semantics.semantics
 ...
         if (isOpen) {
             Box(
                 modifier = Modifier
                     .fillMaxSize()
                     .background(Color.Black.copy(alpha = 0.5f))
-                    .clickable(onClick = onClose),
+                    .clickable(onClick = onClose)
+                    .semantics { onClick(label = "Close drawer", action = { onClose(); true }) },
             )
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/GortDrawer.kt` around
lines 35 - 41, The scrim Box currently uses Modifier.clickable(onClick =
onClose) without accessibility metadata; update the Box's modifier chain in
GortDrawer.kt to add semantics for screen readers by attaching a semantics block
that sets a role (e.g., Role.Button) and a contentDescription (e.g., "Dismiss
overlay" or "Close") and expose the onClose action via an onClick semantic so
assistive tech can announce and activate the scrim; target the Box with the
clickable and onClose handler when adding the semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt`:
- Around line 135-140: The Image used for the theme toggle (Image with
imageVector and modifier using Modifier.size(22.dp).clickable(onClick =
onToggleDark)) is missing an explicit semantic role; update the clickable
modifier to include role = Role.Button so assistive tech announces it as a
button, e.g. modify the Modifier.clickable call on the Image to pass the role
parameter (keeping onToggleDark as the onClick handler) and
import/androidx/compose/ui/semantics Role if needed.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/GortDrawer.kt`:
- Around line 35-41: The scrim Box currently uses Modifier.clickable(onClick =
onClose) without accessibility metadata; update the Box's modifier chain in
GortDrawer.kt to add semantics for screen readers by attaching a semantics block
that sets a role (e.g., Role.Button) and a contentDescription (e.g., "Dismiss
overlay" or "Close") and expose the onClose action via an onClick semantic so
assistive tech can announce and activate the scrim; target the Box with the
clickable and onClose handler when adding the semantics.

In `@gort/src/commonMain/kotlin/zone/clanker/gort/components/GortScaffold.kt`:
- Around line 25-28: GortScaffold's content lambda is being called with an empty
PaddingValues() and snackbarHost() is placed in the same Box as content; compute
and pass real PaddingValues derived from the measured heights of the topBar and
bottomBar slots (measure topBar and bottomBar in the layout, calculate inset
padding (top = topBarHeight, bottom = bottomBarHeight) and pass that into
content(paddingValues)) and ensure snackbarHost() is rendered above content by
placing it in a separate Layer/Box with higher z-index or using Modifier.zIndex
on the snackbarHost composable; update the GortScaffold implementation (look for
the functions/slots named GortScaffold, content, topBar, bottomBar,
snackbarHost, and the Box containing content) to perform these changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6f54355-00ab-402a-9bad-14e2f75bd56c

📥 Commits

Reviewing files that changed from the base of the PR and between 0be009f and da1e37b.

📒 Files selected for processing (10)
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/CatalogApp.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/DataScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/DisplayScreen.kt
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/NavigationScreen.kt
  • gort/build.gradle.kts
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/GortDrawer.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/GortScaffold.kt
  • gort/src/commonMain/kotlin/zone/clanker/gort/components/Spinner.kt
  • gradle/libs.versions.toml
🚧 Files skipped from review as they are similar to previous changes (3)
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/NavigationScreen.kt
  • gort/build.gradle.kts
  • catalog/src/commonMain/kotlin/zone/clanker/gort/catalog/ButtonsScreen.kt

…OPSX proposals

- Add :icons module with 1666 Lucide icon files (zone.clanker.gort.icons.lucide)
- Full platform coverage: Android, JVM, WASM, JS, iOS, macOS native (arm64, x64)
- Publishing config as zone.clanker:gort-icons with useGpgCmd() signing
- :gort depends on libs.gortIcons (Maven coordinate) with dependency substitution
- settings.gradle.kts substitutes zone.clanker:gort-icons -> project(:icons) for local dev
- Drop external com.composables:icons-lucide-cmp dependency entirely
- Migrate all imports to zone.clanker.gort.icons.lucide
- AGP bumped 8.7.3 -> 8.9.3 for Kotlin 2.3.0 compatibility
- Add .claude/ commands and skills from OPSX sync
- Add OPSX proposals: catalog-overhaul, icons-publishing, split-catalog-app, charts
- Add Severity enum, TableControls, LinearProgress, MarkdownScreen
- Extract CatalogSection to separate file
- Add PlatformBackHandler for all targets
- Gort icon SVG added
These are per-developer files that belong in global gitignore,
not in the repo. Commands and skills are regenerated by opsx-sync.
- MarkdownRenderer: full GFM (h1-h6, tables, checkboxes, callouts, nested quotes, links, images)
- Table: sort, pagination, expand/collapse, ShadowDrop controls, haptic feedback
- Timeline: vertical/horizontal, direction toggle, expand, icon support
- ChatBubble: ChatMessage/ToolCall models, ChatView, OmniInputBar with mode detection
- Code: dark header, file path, copy/play icons, SelectionContainer
- TextField: label, helper/error text, password toggle, leading/trailing icons, 48dp min
- FileTree: Lucide icons by type, animated chevron, connector lines, haptic
- Rating: Canvas-drawn stars, gold fill, half-star support
- Tag: unified with Chip, display/filter/removable/disabled modes
- SegmentedControl: SubcomposeLayout, animated sliding indicator, spring bounce
- Stepper: vertical layout with numbered circles, connector lines, checkmarks
- ChoiceGroup: animated border/bg/checkmark with spring
- Callout: thick accent left border, severity-based colors
- Pagination: animateColorAsState with spring bounce, haptic
- IconButton: matches Button shadows/border, 48dp min touch target
- CatalogSection: add Markdown (10th section)
- CatalogApp: Chat fullscreen mode (hides bottom nav), onBack threading
- ChatScreen: full ChatView with sample conversation, markdown, tool calls, CommandPalette
- DataScreen: fullscreen overlay for tables/timelines, paginated/wide table demos, horizontal timeline
- InputsScreen: complete rewrite showcasing all input components with code samples
- MarkdownScreen: already existed, kept as-is
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.

1 participant