feat(#446): plugins tab full parity — install from URL, provider selection, remove confirm, badges (#456)
* docs: document HermesScaffold padding foot-gun root cause in AGENTS.md * feat(#446): plugins tab full parity — install from URL, provider selection, remove confirm, badges - Add Install from URL section with force reinstall + enable after install toggles - Add Plugin Provider selection section (memory provider + context engine dropdowns) - Add rescan button in top bar actions - Add confirmation dialog before removing plugins - Show source badges, runtime status, auth badges on plugin cards - Show dashboard slots from plugin manifest - Add visibility toggle (show/hide in sidebar) for manifest plugins - Show orphan dashboard plugins section - Add PluginManifestData, PluginsHubProviders, ProviderOption models - Add rescanPlugins, savePluginProviders, setPluginVisibility API endpoints * fix(#446): fix CI compilation errors — DashboardPluginMeta mapping and @Composable context * fix(#446): pass missing PluginInfo ctor params for orphan plugin mapping * fix(#446): remove duplicate refresh button in plugins top bar Scaffold's onRefresh param added a second refresh icon alongside the rescan action button. Removed onRefresh/isRefreshing from HermesScaffold and moved pull-to-refresh into a manual PullToRefreshBox wrapping the content, keeping the rescan icon as the sole top-bar action. * fix(#446): remove rescan icon from plugins top bar Only one action icon left in top bar; pull-to-refresh (loadPlugins) remains the sole refresh mechanism. rescanPlugins() viewmodel logic is unchanged but now unreachable from the UI. --------- Co-authored-by: M57 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
0ee24144c0
commit
57a815c67e
6 files changed
+881
-109
No files matched your search
@@ -122,6 +122,55 @@ call unconditionally.
|
||||
- **`StateViews`** — `LoadingState`, `ErrorState`, `EmptyState`. Every data screen
|
||||
must implement all three branches in its `when { }` block.
|
||||
|
||||
### ⚠ HermesScaffold Padding Foot-Gun
|
||||
|
||||
**This is the #1 recurring bug in this codebase.** It has been re-introduced on 4+ screens
|
||||
across 3+ PRs (Settings, Achievements, Webhooks, Config — PRs #445, #454, #455).
|
||||
|
||||
**Root cause:** `HermesScaffold` wraps content in an internal `Box(Modifier.padding(paddingValues))`
|
||||
that already offsets for the top bar. But it also passes `paddingValues` into the content lambda,
|
||||
which looks like it should be applied — and every new screen does exactly that:
|
||||
|
||||
```kotlin
|
||||
HermesScaffold(...) { paddingValues -> // ← scaffold already handles top bar offset via Box
|
||||
LazyColumn(
|
||||
modifier = Modifier
|
||||
.fillMaxSize()
|
||||
.padding(paddingValues), // ← ❌ double-stacked top padding
|
||||
contentPadding = listContentPadding, // ← extra 8dp, now triple-stacked
|
||||
) { ... }
|
||||
}
|
||||
```
|
||||
|
||||
**The deeper issue:** Passing `paddingValues` into the lambda implies "you need to use this,"
|
||||
when the scaffold has ALREADY pre-applied it in its own outer `Box`. This API design creates
|
||||
a natural foot-gun: every developer instinctively adds `.padding(paddingValues)` on inner
|
||||
content because it seems correct.
|
||||
|
||||
**Correct pattern — do NOT apply `paddingValues` on inner content:**
|
||||
|
||||
```kotlin
|
||||
// ✅ List screens:
|
||||
LazyColumn(
|
||||
modifier = Modifier.fillMaxSize(), // no .padding(paddingValues)!!
|
||||
contentPadding = listContentPadding, // this is the ONLY padding needed
|
||||
verticalArrangement = listItemSpacing,
|
||||
)
|
||||
|
||||
// ✅ Non-list screens (Column/Box wrapping):
|
||||
Column(
|
||||
modifier = Modifier.fillMaxSize(), // no .padding(paddingValues)!!
|
||||
) { ... }
|
||||
|
||||
// ✅ Loading/Error/Empty states — these DO need it:
|
||||
LoadingState(modifier = Modifier.padding(paddingValues))
|
||||
```
|
||||
|
||||
**Quick test:** If your screen's top gap is wider than CronJobsScreen's, you've double-stacked.
|
||||
|
||||
**See also:** `references/hermes-scaffold-padding.md` in the skill doc for the full
|
||||
breakdown, edge cases, and timeline of previous occurrences.
|
||||
|
||||
### Room Persistence
|
||||
|
||||
- `ChatMessageEntity` / `ChatMessageDao` / `HermesDatabase` — chat messages survive
|
||||
|
||||
Reference in new issue
Block a user