[Web Import] Add persistent settings for font customization#899
Merged
[Web Import] Add persistent settings for font customization#899
Conversation
WalkthroughThis PR renames Material symbol types (FontSettings → MaterialFontSettings, IconFontFamily → MaterialIconFontFamily) and updates their usages across UI, view models, repositories, and use cases. It adds Material font state properties plus lucide/bootstrap size properties to PersistentSettings, introduces a generic readState helper in InMemorySettings, and wires InMemorySettings into use cases via DI to persist font and size settings. It removes logging from StandardIconViewModel and replaces the deleted FontSettings file with MaterialFontSettings and a revised MaterialIconFontFamily enum. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt (1)
22-23:isModifiedcheck forfillis implicitly coupled toDEFAULT_FILL = false.Using
fill(truthy check) instead offill != DEFAULT_FILLworks only becauseDEFAULT_FILLisfalse. If the default ever changes totrue, this will silently report modifications incorrectly. Consider usingfill != DEFAULT_FILLfor consistency with the other fields.♻️ Suggested change
val isModified: Boolean - get() = fill || weight != DEFAULT_WEIGHT || grade != DEFAULT_GRADE || opticalSize != DEFAULT_OPTICAL_SIZE + get() = fill != DEFAULT_FILL || weight != DEFAULT_WEIGHT || grade != DEFAULT_GRADE || opticalSize != DEFAULT_OPTICAL_SIZE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt` around lines 22 - 23, The isModified getter currently uses a truthy check for fill which assumes DEFAULT_FILL == false; change the comparison to explicitly compare fill != DEFAULT_FILL so it is consistent with the other fields (weight != DEFAULT_WEIGHT, grade != DEFAULT_GRADE, opticalSize != DEFAULT_OPTICAL_SIZE) and will remain correct if DEFAULT_FILL changes; update the isModified expression to use fill != DEFAULT_FILL alongside the existing comparisons referencing DEFAULT_WEIGHT, DEFAULT_GRADE, and DEFAULT_OPTICAL_SIZE.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kt (1)
23-24: Consider whethercommon/ui/SidePanelpreview should depend onmaterial/types.The
SidePanelcomposable is generic (acceptscontent:@composable() -> Unit), but its preview importsMaterialFontCustomizationandMaterialFontSettingsfrom thematerialpackage. This creates a dependency from a common UI component's preview on a specific feature module. A simpler placeholdercontent(e.g., a plainText) would keep this preview independent.This is a pre-existing pattern (just renamed), so no action needed now.
Also applies to: 84-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kt` around lines 23 - 24, The SidePanel preview currently depends on material-specific types (MaterialFontCustomization, MaterialFontSettings); replace that preview content with a minimal placeholder (e.g., a plain Text composable) and remove the imports of MaterialFontCustomization and MaterialFontSettings so the SidePanel preview no longer references the material/ feature module; update both preview usages (the one around SidePanel and the second instance referenced at lines 84-86) to use the simple placeholder content instead.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kt (1)
180-192: Minor dispatcher inconsistency withLucideViewModel.updateSettings.
LucideViewModel.updateSettings(line 154) launches withDispatchers.Default, whileMaterialSymbolsViewModel.updateFontSettingsuses the default dispatcher (Main). Both work fine sinceinMemorySettings.updateis lightweight, but it's a small inconsistency.♻️ Align dispatcher usage
fun updateFontSettings(fontSettings: MaterialFontSettings) { - viewModelScope.launch { + viewModelScope.launch(Dispatchers.Default) { inMemorySettings.update {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kt` around lines 180 - 192, The two view models use different coroutine dispatchers: MaterialSymbolsViewModel.updateFontSettings currently uses viewModelScope.launch (Main) while LucideViewModel.updateSettings uses viewModelScope.launch(Dispatchers.Default); make them consistent by launching updateFontSettings with viewModelScope.launch(Dispatchers.Default) and perform inMemorySettings.update and updateSuccess within that dispatcher (keeping function names MaterialSymbolsViewModel.updateFontSettings, inMemorySettings.update, and updateSuccess to locate the changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kt`:
- Around line 23-24: The SidePanel preview currently depends on
material-specific types (MaterialFontCustomization, MaterialFontSettings);
replace that preview content with a minimal placeholder (e.g., a plain Text
composable) and remove the imports of MaterialFontCustomization and
MaterialFontSettings so the SidePanel preview no longer references the material/
feature module; update both preview usages (the one around SidePanel and the
second instance referenced at lines 84-86) to use the simple placeholder content
instead.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt`:
- Around line 22-23: The isModified getter currently uses a truthy check for
fill which assumes DEFAULT_FILL == false; change the comparison to explicitly
compare fill != DEFAULT_FILL so it is consistent with the other fields (weight
!= DEFAULT_WEIGHT, grade != DEFAULT_GRADE, opticalSize != DEFAULT_OPTICAL_SIZE)
and will remain correct if DEFAULT_FILL changes; update the isModified
expression to use fill != DEFAULT_FILL alongside the existing comparisons
referencing DEFAULT_WEIGHT, DEFAULT_GRADE, and DEFAULT_OPTICAL_SIZE.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kt`:
- Around line 180-192: The two view models use different coroutine dispatchers:
MaterialSymbolsViewModel.updateFontSettings currently uses viewModelScope.launch
(Main) while LucideViewModel.updateSettings uses
viewModelScope.launch(Dispatchers.Default); make them consistent by launching
updateFontSettings with viewModelScope.launch(Dispatchers.Default) and perform
inMemorySettings.update and updateSuccess within that dispatcher (keeping
function names MaterialSymbolsViewModel.updateFontSettings,
inMemorySettings.update, and updateSuccess to locate the changes).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
tools/idea-plugin/CHANGELOG.mdtools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/service/PersistentSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/settings/InMemorySettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/lucide/LucideViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsImportScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/MaterialSymbolsConfigUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialIconFontFamily.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontCustomization.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontFamilyDropdown.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialTopActions.kt
💤 Files with no reviewable changes (1)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kt
egorikftp
force-pushed
the
feature/idea/material-save-settings
branch
from
9aff2cb to
0855840
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kt (1)
213-221: Add default value tofontSettingsparameter inMaterialState.Success
fontSettings: MaterialFontSettingshas no default value while surrounded byselectedCategory(which defaults toCategory.All) andiconFontFamily(which defaults toMaterialIconFontFamily.Outlined). Since all fields ofMaterialFontSettingscarry default values,MaterialFontSettings()is a valid no-arg call. Adding a default here aligns with Kotlin conventions for parameter ordering.Proposed fix
- val fontSettings: MaterialFontSettings, + val fontSettings: MaterialFontSettings = MaterialFontSettings(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kt` around lines 213 - 221, The Success variant of MaterialState declares fontSettings: MaterialFontSettings without a default while neighboring params have defaults; update the data class MaterialState.Success to give fontSettings a default value by using MaterialFontSettings() so callers can omit it (i.e., change the constructor param in Success to fontSettings: MaterialFontSettings = MaterialFontSettings()) and keep other defaults (selectedCategory, iconFontFamily, fontByteArray) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kt`:
- Around line 213-221: The Success variant of MaterialState declares
fontSettings: MaterialFontSettings without a default while neighboring params
have defaults; update the data class MaterialState.Success to give fontSettings
a default value by using MaterialFontSettings() so callers can omit it (i.e.,
change the constructor param in Success to fontSettings: MaterialFontSettings =
MaterialFontSettings()) and keep other defaults (selectedCategory,
iconFontFamily, fontByteArray) unchanged.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
tools/idea-plugin/CHANGELOG.mdtools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/service/PersistentSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/settings/InMemorySettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/lucide/LucideViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsImportScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/MaterialSymbolsConfigUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialIconFontFamily.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontCustomization.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontFamilyDropdown.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialTopActions.kt
💤 Files with no reviewable changes (1)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- tools/idea-plugin/CHANGELOG.md
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/service/PersistentSettings.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialIconFontFamily.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/MaterialSymbolsConfigUseCase.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialTopActions.kt
egorikftp
force-pushed
the
feature/idea/material-save-settings
branch
from
0855840 to
e3a8917
Compare
3 tasks
t-regbs
reviewed
Feb 25, 2026
.../io/github/composegears/valkyrie/ui/screen/webimport/standard/domain/StandardIconProvider.kt
Outdated
Show resolved
Hide resolved
t-regbs
reviewed
Feb 25, 2026
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/settings/InMemorySettings.kt
Show resolved
Hide resolved
t-regbs
requested changes
Feb 25, 2026
egorikftp
force-pushed
the
feature/idea/material-save-settings
branch
from
e3a8917 to
6a6a96c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kt (1)
32-50:⚠️ Potential issue | 🟠 MajorHandle empty icon option when only optical size changes.
Line 33 allows the modified path, but Lines 38-49 can still return an empty option when only
opticalSizediffers. That leads to an invalid URL segment and failed SVG download.Proposed fix
private fun MaterialFontSettings.iconOption(): String { if (!isModified) { return "default" } - return buildString { + val option = buildString { if (weight != 400) { append("wght$weight") } if (grade != 0) { when { grade < 0 -> append("gradN${abs(grade)}") else -> append("grad$grade") } } if (fill) { append("fill1") } } + return option.ifEmpty { "default" } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kt` around lines 32 - 50, In MaterialFontSettings.iconOption(), when isModified is true the buildString block can still produce an empty string (for example if only opticalSize changed), which yields an invalid URL segment; update the method so that after constructing the option string (using weight, grade, fill, etc.) you check if the built string is blank/empty and return "default" in that case (instead of an empty string); refer to the MaterialFontSettings.iconOption function and the variables weight, grade, fill, opticalSize and isModified to implement this guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/domain/BootstrapUseCase.kt`:
- Around line 23-29: persistentSize is currently a one-time initialized val
using inMemorySettings.readState and becomes stale after updatePersistentSize
mutates state; change persistentSize to a getter (e.g., override val
persistentSize: Int get() = inMemorySettings.readState { bootstrapSize }) so
every access reads the current state, and apply the same change for the
analogous property in LucideUseCase; keep updatePersistentSize implementation
(which calls inMemorySettings.update { bootstrapSize = value }) unchanged.
---
Outside diff comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kt`:
- Around line 32-50: In MaterialFontSettings.iconOption(), when isModified is
true the buildString block can still produce an empty string (for example if
only opticalSize changed), which yields an invalid URL segment; update the
method so that after constructing the option string (using weight, grade, fill,
etc.) you check if the built string is blank/empty and return "default" in that
case (instead of an empty string); refer to the MaterialFontSettings.iconOption
function and the variables weight, grade, fill, opticalSize and isModified to
implement this guard.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
tools/idea-plugin/CHANGELOG.mdtools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/service/PersistentSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/settings/InMemorySettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsImportScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/MaterialSymbolsConfigUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialIconFontFamily.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontCustomization.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontFamilyDropdown.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialTopActions.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/StandardIconViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/di/BootstrapModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/domain/BootstrapUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/domain/StandardIconProvider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/di/LucideModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/domain/LucideUseCase.kt
💤 Files with no reviewable changes (1)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt
- tools/idea-plugin/CHANGELOG.md
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialTopActions.kt
...thub/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/domain/BootstrapUseCase.kt
Show resolved
Hide resolved
t-regbs
approved these changes
Feb 25, 2026
egorikftp
force-pushed
the
feature/idea/material-save-settings
branch
from
6a6a96c to
82f05eb
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kt (1)
32-51:⚠️ Potential issue | 🔴 Critical
iconOption()can return an empty string, producing a malformed URL when onlyopticalSizeis non-default.
isModifiedincludesopticalSizein its check, so if the user changes only optical size (e.g.fill=false,weight=400,grade=0,opticalSize=36f),isModifiedistrueand the"default"early-return is skipped. But thebuildStringblock only handlesweight,grade, andfill— none of which deviate — so it produces an emptyString. The resulting URL becomes:https://fonts.gstatic.com/s/i/short-term/release/$fontFamily/$name//$iconSize.svg...with a double slash, which is an invalid resource path.
🐛 Proposed fix — fall back to `"default"` when the built string is empty
- private fun MaterialFontSettings.iconOption(): String { - if (!isModified) { - return "default" - } - - return buildString { - if (weight != 400) { - append("wght$weight") - } - if (grade != 0) { - when { - grade < 0 -> append("gradN${abs(grade)}") - else -> append("grad$grade") - } - } - if (fill) { - append("fill1") - } - } - } + private fun MaterialFontSettings.iconOption(): String { + return buildString { + if (weight != DEFAULT_WEIGHT) { + append("wght$weight") + } + if (grade != DEFAULT_GRADE) { + when { + grade < 0 -> append("gradN${abs(grade)}") + else -> append("grad$grade") + } + } + if (fill) { + append("fill1") + } + }.ifEmpty { "default" } + }This removes the
isModifiedgate (which incorrectly includesopticalSize) and instead letsbuildStringrun always, falling back to"default"only when the relevant options produce an empty string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kt` around lines 32 - 51, MaterialFontSettings.iconOption() can produce an empty string (causing a malformed URL) when only opticalSize is changed; fix by ensuring the method returns "default" whenever the constructed string is empty: run the existing buildString logic (or keep the current isModified check) but after building the string check if the result is blank/empty and return "default" in that case; update the MaterialFontSettings.iconOption() implementation so callers (e.g., URL construction using iconOption()) never receive an empty string.
🧹 Nitpick comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt (1)
5-27: LGTM — clean model with sensible defaults and range validation.One note: the
isModifiedgetter at Line 26 usesfill ||as a shorthand forfill != DEFAULT_FILL(which holds becauseDEFAULT_FILL = false). This is correct, but it's worth knowing that this expression tiesisModifiedsemantics to the specific default value offill. IfDEFAULT_FILLever changes totrue,isModifiedwould silently break (a non-filled state would incorrectly appear as "modified" when fill == true is the new default). Considerfill != DEFAULT_FILL || ...for robustness.✏️ Suggested fix for future-proofing `isModified`
- override val isModified: Boolean - get() = fill || weight != DEFAULT_WEIGHT || grade != DEFAULT_GRADE || opticalSize != DEFAULT_OPTICAL_SIZE + override val isModified: Boolean + get() = fill != DEFAULT_FILL || weight != DEFAULT_WEIGHT || grade != DEFAULT_GRADE || opticalSize != DEFAULT_OPTICAL_SIZE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt` around lines 5 - 27, Update the isModified getter in MaterialFontSettings to compare fill against DEFAULT_FILL explicitly (use fill != DEFAULT_FILL) instead of relying on the current boolean literal; keep the other comparisons (weight != DEFAULT_WEIGHT, grade != DEFAULT_GRADE, opticalSize != DEFAULT_OPTICAL_SIZE) as-is so isModified remains robust if DEFAULT_FILL changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kt`:
- Around line 32-51: MaterialFontSettings.iconOption() can produce an empty
string (causing a malformed URL) when only opticalSize is changed; fix by
ensuring the method returns "default" whenever the constructed string is empty:
run the existing buildString logic (or keep the current isModified check) but
after building the string check if the result is blank/empty and return
"default" in that case; update the MaterialFontSettings.iconOption()
implementation so callers (e.g., URL construction using iconOption()) never
receive an empty string.
---
Nitpick comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kt`:
- Around line 5-27: Update the isModified getter in MaterialFontSettings to
compare fill against DEFAULT_FILL explicitly (use fill != DEFAULT_FILL) instead
of relying on the current boolean literal; keep the other comparisons (weight !=
DEFAULT_WEIGHT, grade != DEFAULT_GRADE, opticalSize != DEFAULT_OPTICAL_SIZE)
as-is so isModified remains robust if DEFAULT_FILL changes.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
tools/idea-plugin/CHANGELOG.mdtools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/service/PersistentSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/settings/InMemorySettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsImportScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/MaterialSymbolsViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/data/font/MaterialFontRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/MaterialSymbolsConfigUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialFontSettings.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialIconFontFamily.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontCustomization.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontFamilyDropdown.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialTopActions.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/StandardIconViewModel.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/di/BootstrapModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/domain/BootstrapUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/domain/StandardIconProvider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/di/LucideModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/domain/LucideUseCase.kt
💤 Files with no reviewable changes (1)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/FontSettings.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- tools/idea-plugin/CHANGELOG.md
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/di/BootstrapModule.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/di/LucideModule.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/MaterialSymbolsConfigUseCase.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/domain/BootstrapUseCase.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/MaterialFontCustomization.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/domain/model/font/MaterialIconFontFamily.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Screen.Recording.2026-02-24.at.18.19.01.mov
📝 Changelog
If this PR introduces user-facing changes, please update the relevant Unreleased section in changelogs: