Issue #91 — LEDSpicerUI layout bugs — changes made#94
Merged
Conversation
Task-Url: #91 Bug fixes: - Random colors list no longer carries over stale entries between project loads (boxRandomColors is now wiped unconditionally before repopulating). - Settings > General tab: Switch/Label order swapped to match the rest of the app (Label first, then Switch), and labels given expand=True so all switches align flush-right within their row. - Removed 3 dead `.clear()` calls in readConfigFile (redundant with load()'s own internal clear). - Removed a stale, contentless TODO comment in DialogLinkEditor.cpp. - Re-selecting the already-open project now offers a real revert (discard unsaved changes + reload from disk) instead of just an info message, when the project is dirty. Save-time "All" group optimization: - ConfigFile::save() now omits the "All" group from the saved file when it exactly matches the default device order (position-for-position), since the daemon auto-builds it identically in that case. - Strip elements are expanded into their children (not the unlinkable parent) when building that comparison, matching daemon behavior. - xmlSection() gained an `allowEmpty` parameter so `<layout>`'s defaultProfile/defaultProject attributes still get written even when the groups body ends up empty after "All" is omitted. Code quality / review follow-ups: - Moved ProjectFile.hpp / Storage/Element.hpp includes into ConfigFile.hpp (header carries the deps, not the .cpp). - Multi-line comments in ConfigFile.cpp switched from stacked `//` to `/* */`. - Extracted the strip-expansion and order-comparison logic out of save() into two named, testable static methods: expandDeviceElements() and matchesDeviceOrder(). Tests: - Added 7 unit tests covering both extracted helpers: plain element order, strip-parent exclusion (children only, correct position), mixed plain+strip sequencing, and matching cases for matchesDeviceOrder (identical, reordered, size mismatch, strip children reordered). - Added tests/mocks/MockDevice.hpp (reusable minimal Parent stand-in for a device), matching the existing MockData/MockBasicData conventions. - Verified with a deliberate negative control (temporarily broke matchesDeviceOrder) to confirm the new tests actually catch the regression they're meant to catch, not passing vacuously. Investigated, found not to be bugs: - "Default profile not selected after loading" — confirmed to be expected behavior in Local Mode, where projects share one physical ledspicer.conf file (not a defect). - Cascade delete on Element removal (dependency system correctly purges stale Group links, including in "All", when devices are wiped) — verified correct, no dangling pointers.
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
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.
Task-Url: #91
Bug fixes:
.clear()calls in readConfigFile (redundant with load()'s own internal clear).unsaved changes + reload from disk) instead of just an info message, when the project is dirty.
Save-time "All" group optimization:
allowEmptyparameter so<layout>'s defaultProfile/defaultProject attributes still get written even when the groups body ends up empty after "All" is omitted.Code quality / review follow-ups:
//to/* */.Tests:
Investigated, found not to be bugs: