PICARD-3196: Implement custom persistence for column state in ConfigurableColumnsHeader#3043
Conversation
…rableColumnsHeader Instead of relying on an opaque binary structure based on logical column indices use a custom structure which stores the state of each column based on its name. This fixes issues with column state breaking if the indices of the default columns change, e.g. when new default columns are addded or removed. As there is not clean upgrade path there is a config upgrade to remove the old states. The columns will reset to their default state on upgrade.
|
I found some other issue: After unlocking and then adding / removing columns the header does not directly update. Only after clicking on the header again. It's rather strange, but I cannot reproduce on master. |
Specifically setting the width for the last "stretch" column broke adding new columns to the view.
phw
force-pushed
the
PICARD-3196-fix-column-persistence
branch
from
12a85ae to
e2e1216
Compare
|
@zas Please test again. I fixed the issues we discussed above. I re-implemented the lock handling to not interfere with the default Qt based state restore. The issue about not being able to add new columns after restore was caused by setting the width for all columns. If you run |
Actually found some issues when reviewing and testing. |
The restore_columns_state() method had a critical bug that prevented
correct restoration of column positions:
Columns were repositioned during iteration in logical index order, but
Qt's moveSection() operates on visual indices. Each move changes the
visual positions of other columns, causing cascading displacement that
prevented correct restoration of the saved layout.
This commit refactors the method into two clear phases:
Phase 1: Restore visibility, resize modes, widths, and sort indicator
- Must be done first as visibility/resize modes affect moveSection()
- Sort indicator uses logical indices, independent of positioning
Phase 2: _restore_column_positions()
- Processes target positions in ascending order (0, 1, 2, ...)
- This ensures each column reaches its final position without being
displaced by subsequent moves
- Also fixes position 0 exclusion (currently not an issue since first
columns are unmovable, but future-proofs the code)
The code includes inline comments explaining Qt's behavior and the
rationale for the specific ordering.
Also cleaned up redundant `.get(column.key, None)` calls to use the
default `.get(column.key)` behavior.
2 participants
Summary
Problem
Changing the default columns for the main views can break the column display as the persisted state does not match the column model anymore. See e.g. #3040 and the example screenshot there.
Solution
Instead of relying on an opaque binary structure based on logical column indices use a custom structure which stores the state of each column based on its name. This fixes issues with column state breaking if the indices of the default columns change, e.g. when new default columns are addded or removed.
As there is not clean upgrade path there is a config upgrade to remove the old states. The columns will reset to their default state on upgrade.
AI Usage
In accordance with the AI use policy portion of the MetaBrainz Contribution Guidelines, the level of AI/LLM use in the development of this Pull Request is:
Action
Additional actions required: