feat: redesign options UI for natural UX flow (#124)#144
Conversation
- Rename "Loot Roll" tab to "Roll Frame" - Split overloaded Layout section in LootRollTab into four focused sections: Frame, Timer Bar, Buttons, Icon - Add contextual disabling in LootRollTab: all layout widgets disabled when Enable Custom Roll Frame is off; sub-state (compact mode, timer bar style, border, color mode, icon position) correctly re-applied on re-enable via reapplySubState closure pattern - Add contextual disabling in LootWindowTab: Lock Position, Position at Cursor, and layout sliders disabled when Enable Custom Loot Window off - Restructure HistoryTab into three sections: History (master controls), Recording (what to capture), Display (how it looks); add missing Lock Position toggle for history frame - Rename AutoLootTab Settings section (was "Smart Auto-Loot"); add contextual disabling for Minimum Quality when auto-loot is off - Rename AnimationTab "Animation" section to "Global Settings"; add contextual disabling for duration sliders and animation dropdowns when animations are off - Add 9 new enUS locale keys for new section headers and tooltips No config key changes; purely presentation/layout.
📝 WalkthroughWalkthroughThis pull request restructures the options UI across multiple tabs, introducing clearer section groupings that align settings by purpose rather than implementation. New locale keys are added, and a pattern of contextual enable/disable is applied where dependent controls are disabled when their parent toggle is off. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
DragonLoot_Options/Tabs/LootRollTab.lua (1)
665-684: Initial-state logic matches the runtime handler — nice.Worth noting that when
enabled == truethe individual creation-timeSetDisabledcalls (lines 237, 264, 347, 366, 426, 463, 608) already set correct sub-state, so thereapplySubStateloop here is a benign redundancy. If you want to trim a few cycles you could drop one of the two, but it's harmless as-is and keeps the init path symmetric with the toggle path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DragonLoot_Options/Tabs/LootRollTab.lua` around lines 665 - 684, The init path re-applies sub-state redundantly: the CreateRollFrameSection/CreateFrameSection/CreateTimerBarSection/CreateButtonsSection/CreateIconSection functions already call widget:SetDisabled(...) during creation, so when db.profile.rollFrame.enabled is true you can remove the reapplySubState loop; update the conditional after creating layoutWidgets so the "else" branch skips the for _, fn in ipairs(reapplySubState) do fn() end loop (leaving the initial disabled-state loop when enabled == false intact) to avoid duplicate sub-state work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@DragonLoot_Options/Tabs/LootRollTab.lua`:
- Around line 665-684: The init path re-applies sub-state redundantly: the
CreateRollFrameSection/CreateFrameSection/CreateTimerBarSection/CreateButtonsSection/CreateIconSection
functions already call widget:SetDisabled(...) during creation, so when
db.profile.rollFrame.enabled is true you can remove the reapplySubState loop;
update the conditional after creating layoutWidgets so the "else" branch skips
the for _, fn in ipairs(reapplySubState) do fn() end loop (leaving the initial
disabled-state loop when enabled == false intact) to avoid duplicate sub-state
work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 0831a433-5acf-440e-b23a-193c93393ecd
📒 Files selected for processing (6)
DragonLoot/Locales/enUS.luaDragonLoot_Options/Tabs/AnimationTab.luaDragonLoot_Options/Tabs/AutoLootTab.luaDragonLoot_Options/Tabs/HistoryTab.luaDragonLoot_Options/Tabs/LootRollTab.luaDragonLoot_Options/Tabs/LootWindowTab.lua
Summary
Redesigns the options UI across all tabs for a more natural, intuitive UX flow. Pure presentation changes - no config key names or defaults are modified.
Changes
Roll Frame tab (was "Loot Roll")
Loot Window tab
History tab
Auto-Loot tab
Animation tab
Testing
luacheck .passes with 0 new warningsCloses #124
Summary by CodeRabbit
New Features
Improvements