Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix selection expanded after change. #2124

Conversation

oliver-sanders
Copy link
Member

The expanded selection was incorrectly updated in the event that the TreeStore was modified in a manner which caused a re-ordering of rows which in-turn affected a re-ordering of sections. This is a presentation layer bug as the underlying data-structure is not affected.

The steps to reproduce are somewhat convoluted:

  1. Open a widget which inherits from the BaseSummaryDataPanel class.
    • Note: The data-structure will require at least 4 columns.
    • Note: At least three sections must be present.
  2. Group by one column.
  3. Expand a section within it ensuring that:
    • The section has 3 or more items.
    • In the section, at least two columns where at least two values are present.
    • The section is not the first in the panel.
  4. Sort by an even number of columns (at least one of which must contain multiple values).
  5. Change a value in one of the remaining columns (multiple values must be present) to the lowest possible value (this way it is certain to change the sorting).

Note: The bug applies to the BaseSummaryDataPanel class (from which BaseStashSummaryDataPanelv1 inherits).

@oliver-sanders oliver-sanders self-assigned this Oct 23, 2017
@oliver-sanders
Copy link
Member Author

Test failure fixed by #2123.

@oliver-sanders oliver-sanders added this to the next-release milestone Oct 23, 2017
@oliver-sanders
Copy link
Member Author

@arjclark Can you confirm this resolves the issue you reported?

@arjclark
Copy link
Contributor

@oliver-sanders - tested and looks like this keeps the correct group open.

Although the ordering of the groups themselves changes (so if you have two similar groups open you may miss the redraw), this change does address the immediate gotcha of ungrouping things that weren't there previously. I'm not sure how hard/feasible it would be to maintain the ordering of the groupings themselves though?

@oliver-sanders
Copy link
Member Author

I'm not sure how hard/feasible it would be to maintain the ordering of the groupings themselves though?

Short Answer:

Unfortunately sorting is handled internally within GTK. If the re-ordering is an issue the best you can do is to sort by the top level field (in the reported case this would be "packages").

Long Answer:

I had a go at writing a manual iterator to work through the GTK data-structure (gtk.TreeStore via gtk.TreeModel) - one could then manually swap items as desired to effectively define an extra sorting layer. Unfortunately the sorting is handled internally in GTK creating a barrier between the stored and represented data meaning that this approach won't work.

@arjclark
Copy link
Contributor

@oliver-sanders - thanks for the clarification. GTK vs. TreeViews/Stores once again proves a nuisance 🙁

Am happy for this to be the fix to close the reported issue.

@oliver-sanders
Copy link
Member Author

I'll wait for #2123 before merging this.

@oliver-sanders
Copy link
Member Author

./t/syntax/00-lib.t is fixed by #2128

@oliver-sanders oliver-sanders force-pushed the summary-data-panel-fix-expanded-selection branch from 3140de6 to 32b2cf5 Compare November 3, 2017 09:25
@oliver-sanders oliver-sanders merged commit fdd5524 into metomi:master Nov 3, 2017
@oliver-sanders oliver-sanders deleted the summary-data-panel-fix-expanded-selection branch November 20, 2017 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants