Skip to content

[Plugin] Add Bootstrap Icons#858

Draft
t-regbs wants to merge 10 commits intomainfrom
feature/bootstrap-icons
Draft

[Plugin] Add Bootstrap Icons#858
t-regbs wants to merge 10 commits intomainfrom
feature/bootstrap-icons

Conversation

@t-regbs
Copy link
Collaborator

@t-regbs t-regbs commented Feb 13, 2026


@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

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

Adds a Bootstrap colored ImageVector and accessor. Introduces a provider-based web import architecture: new StandardIconProvider interface, StandardIcon models/config, InferredCategory, SizeSettings and IconSettings, CategoryInferrer, SvgSizeCustomizer, StandardGridFiltering, and font conversion utility. Adds Bootstrap and Lucide provider implementations, repositories, use cases, DI modules, and import screens (BootstrapImportScreen, LucideImportScreen, StandardImportScreen). Replaces Lucide-specific UI and domain types with standardized equivalents, adds customization UI (CustomizationToolbar, IconSizeCustomization), and removes the legacy LucideUseCase and related Lucide domain models.

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title '[Plugin] Add Bootstrap Icons' directly and concisely describes the main change: adding Bootstrap Icons support to the plugin, which is the primary focus of the PR.
Description check ✅ Passed The PR description matches the template requirements with all three changelog items marked as complete, indicating the author has followed the repository's changelog update guidelines.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/bootstrap-icons

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/IconSizeCustomization.kt`:
- Around line 35-43: The KDoc for IconSizeCustomization.kt contains a stale
example key "web.import.font.customize.lucide.size" for the sizeLabel parameter;
update the KDoc in the IconSizeCustomization file to use the new key
"web.import.font.customize.size" (update the `@param` sizeLabel example text) so
the documentation matches the renamed resource key and ensure the comment
mentions a generic example rather than the Lucide-specific one.

In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/domain/CategoryInferrer.kt`:
- Around line 20-52: The "key" entry in CATEGORY_KEYWORDS
(CategoryKeyword("key", "Security", priority = 1)) causes substring
false-positives (e.g., "keyboard"); change the keyword to a more specific token
and/or add delimiter-aware matching: replace the CategoryKeyword("key", ...)
with a more precise keyword like "key-" or "padlock" (and/or "lock") and update
the matching code that consumes CATEGORY_KEYWORDS to perform word-boundary or
delimiter (e.g., '-' or '_') based matching instead of raw substring checks so
only exact token matches map to "Security".
🧹 Nitpick comments (9)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/data/BootstrapRepository.kt (2)

55-57: Consider sanitizing or validating iconName before URL interpolation.

While iconName values originate from the parsed JSON keys (so they should be safe in practice), directly interpolating user-selectable strings into URLs without validation is a defensive coding concern. A simple check (e.g., verifying the name matches [a-z0-9-]+) would guard against unexpected input.


20-25: Consider pinning bootstrap-icons to a specific version for reproducible builds.

UNPKG_BASE uses @latest, making the plugin's behavior non-deterministic—if Bootstrap Icons publishes a breaking change, the plugin could break without any code change on your side. However, this pattern is consistent with LucideRepository, which also uses @latest for lucide-static. If pinning is desired, apply it across all icon repositories and use the current version (1.13.1 rather than 1.11.3).

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/CustomizationToolbar.kt (1)

28-38: Consider renaming string resource keys to remove "font" prefix.

The toolbar is reused in both FontCustomization and IconSizeCustomization contexts, but the resource keys web.import.font.customize.header and web.import.font.customize.reset contain "font" in their names. While the displayed text ("Customize", "Reset") is already generic and functions correctly in both contexts, renaming these keys to web.import.customize.header and web.import.customize.reset would improve clarity for future maintainers.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/domain/BootstrapUseCase.kt (1)

25-33: Category inference relies solely on icon name for Bootstrap.

Since Bootstrap icons lack tag metadata (tags = emptyList()), inferCategoryFromTags will only match on icon names. This means many Bootstrap icons may fall into the "General" fallback category if their names don't contain keywords from CATEGORY_KEYWORDS. This may result in a less useful category filter compared to Lucide.

Consider whether this is acceptable UX or whether Bootstrap-specific keyword tuning (or disabling category filtering for Bootstrap) would be worthwhile.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/domain/CategoryInferrer.kt (1)

73-90: Priority boosting for name matches can produce negative priorities.

Line 79: it.copy(priority = it.priority - 2) means priority-1 keywords matched by name become priority -1. This works correctly since lower = higher priority, but the negative values and the implicit coupling between the boost value (-2) and the priority scale (1–5) could be fragile if new priority levels are added.

A minor readability improvement would be to document the effective priority range (or use a named constant for the boost).

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/StandardIconViewModel.kt (1)

154-159: Minor: updateSuccess has a potential read-modify-write race.

selectCategory, updateSearchQuery, and updateSettings all dispatch on Dispatchers.Default and call updateSuccess concurrently. Two concurrent calls can read the same snapshot and one update gets lost. In practice, UI-driven actions are near-sequential so the risk is low, but worth noting.

A Mutex or funneling all updates through a single-threaded dispatcher (e.g., Dispatchers.Main) would eliminate the race.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/StandardImportScreenUI.kt (2)

113-117: Modifier ordering: fillMaxSize() before external modifier.

The external modifier is applied after fillMaxSize(), which means caller-supplied size constraints are ignored. If this is intentional (screen must always fill), consider documenting it; otherwise, swap the order.

♻️ Suggested fix (if caller sizing should be respected)
-        modifier = Modifier
-            .fillMaxSize()
-            .then(modifier),
+        modifier = modifier
+            .fillMaxSize(),

297-301: Unchecked cast: item.icon as StandardIcon.

IconItem<*> is erased at runtime, so this cast is unchecked. It works as long as only StandardIcon items are added to gridItems, but a future misuse could produce a ClassCastException with no compile-time guard.

Consider using a typed IconItem<StandardIcon> alias or a safe-cast with a fallback.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/LucideImportScreen.kt (1)

20-28: navController.parent?.navigate silently no-ops if parent is null.

If parent is ever null, the download completes but the user sees no navigation — a silent failure. This is likely safe given the screen is always nested inside WebImportFlow, but a log or assertion would make debugging easier if the hierarchy changes.

@t-regbs t-regbs force-pushed the feature/bootstrap-icons branch from b28d2ae to b69ae5d Compare February 13, 2026 01:46
) {
companion object {
private const val UNPKG_BASE = "https://unpkg.com/bootstrap-icons@latest"
private const val FONT_URL = "$UNPKG_BASE/font/fonts/bootstrap-icons.woff2"
Copy link
Member

Choose a reason for hiding this comment

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

Does woff2 really work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it does, I wasn't sure myself at first. But this package only has a woff2 and woff, no .ttf. It works fine and I can't think of any issues with it.

Untitled.mov

Copy link
Member

Choose a reason for hiding this comment

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

Wow, in this case, we can switch to woff2 for all icon providers. woff2 font files are half the size of ttf files. Will add a ticket

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@egorikftp Can't find reliable alternative source with ttf font for this. Think we can either:

  • Download the WOFF2 from CDN, convert it to TTF, bundle the TTF with the plugin.
    Just that this is a manual process, need to redo when Bootstrap Icons updates
  • Use SVG rendering instead of font icons
  • I found Brotli4j, we can use this and write custom woff/woff2 to ttf parser in the plugin

Will prototype the third option first on a separate branch to see how feasible it is

Copy link
Member

Choose a reason for hiding this comment

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

one more idea to use native bindings 💀

For example https://github.com/khoben/woff2-android but only for desktop platforms

@t-regbs t-regbs force-pushed the feature/bootstrap-icons branch from b69ae5d to d93bbad Compare February 13, 2026 08:38
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/BootstrapImportScreen.kt`:
- Around line 20-28: The current onIconDownload uses
navController.parent?.navigate which silently drops navigation when parent is
null; update BootstrapImportScreen's onIconDownload to handle a null parent
explicitly by either using a non-null assertion (e.g.,
requireNotNull/navController.parent!!) to fail fast during development or by
checking navController.parent and logging/showing an error before returning,
then calling navigate(dest = SimpleConversionScreen, navArgs = TextSource(...))
on the non-null parent to ensure the user gets feedback instead of a silent
no-op.
🧹 Nitpick comments (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/domain/CategoryInferrer.kt (1)

73-90: Duplicate name+tag matches are harmless but create unnecessary allocations.

If a keyword (e.g., "arrow") matches both the icon name and a tag, it appears twice in allMatches — once boosted, once at original priority. minByOrNull still picks the correct (boosted) entry, so correctness is fine. The extra allocation is negligible for the expected dataset sizes.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/StandardIconViewModel.kt (1)

154-159: Non-atomic read-modify-write on stateRecord.value.

updateSuccess reads the current value, checks the type, and writes back. If two coroutines call this concurrently (e.g., selectCategory on Dispatchers.Default and downloadFont completing simultaneously), the second write can silently overwrite changes from the first. The practical risk is low since user interactions are sequential, but be aware this is inherently racy.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/StandardImportScreenUI.kt (2)

113-116: Modifier ordering: fillMaxSize() is applied before the caller's modifier.

The external modifier is applied via .then(modifier) after .fillMaxSize(), which means callers cannot override the size constraint. Typically, the incoming modifier is applied first. If the intent is to always fill max size, consider documenting this or reordering.

🔧 Suggested reorder
     Column(
-        modifier = Modifier
-            .fillMaxSize()
-            .then(modifier),
+        modifier = modifier
+            .fillMaxSize(),
     ) {

297-303: Unchecked cast item.icon as StandardIcon.

IconItem<*> erases the type parameter, so this cast is unchecked and will only fail at runtime if a non-StandardIcon item is ever added to the grid. This is safe today since the ViewModel only supplies StandardIcon items, but it's fragile.

Consider using a typed alias (e.g., typealias StandardIconItem = IconItem<StandardIcon>) and filtering/casting at the data boundary instead of in the UI layer.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/font/WoffToTtfConverter.kt`:
- Around line 127-129: The loop in WoffToTtfConverter that repeatedly calls
inflater.inflate(buffer) can spin forever if inflate() returns 0 (e.g.,
truncated stream or dictionary required); change the loop to capture the
returned int (e.g., int read = inflater.inflate(buffer)) and if read == 0 then
detect the cause: if inflater.needsDictionary() throw an IOException indicating
a missing preset dictionary, if inflater.needsInput() either feed more input or
throw an IOException for truncated input, otherwise break/throw to avoid an
infinite loop; keep the rest of the inflater.finished() handling intact so you
still write non-zero reads to output.
🧹 Nitpick comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/font/WoffToTtfConverter.kt (1)

149-149: Consider using an integer-based log2 instead of floating-point ln.

Since the argument is always an exact power of 2 (from highestPowerOf2), Integer.numberOfTrailingZeros is exact and avoids any floating-point edge cases.

Proposed fix
-    private fun log2(n: Int): Int = (ln(n.toDouble()) / ln(2.0)).toInt()
+    private fun log2(n: Int): Int = Integer.numberOfTrailingZeros(n)

This also removes the import kotlin.math.ln on line 8.

@egorikftp egorikftp marked this pull request as draft February 19, 2026 10:32
@egorikftp egorikftp marked this pull request as draft February 19, 2026 10:32
@t-regbs
Copy link
Collaborator Author

t-regbs commented Feb 19, 2026

@egorikftp Been exploring the JNI binding idea and I like it. See latests commits on this PR, I added:

  • JNI bridge (~35 lines of C++)
  • Pre-built binaries for macOS (universal arm64+x86_64), Linux x86_64, and Windows x86_64. Think it is safe to just commit in the project and bundle. I am only able to test for macOS though, will need testing on windows and linux.
  • Platform-detecting loader that extracts the correct binary from JAR resources at runtime
  • GitHub Actions workflow (build-native-libs.yml) for rebuilding the native binaries when needed (manual trigger). Shouldn't really need to run this ever since the woff2 spec does not change. It is how I built the windows and linux binaries since I couldn't do it locally, so i just left it in.

Let me know your thoughts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments