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

Fixed ChooserPresenter moving and filtering items #1494

Merged
merged 1 commit into from Dec 19, 2023

Conversation

JanBliznicenko
Copy link
Contributor

@JanBliznicenko JanBliznicenko commented Dec 15, 2023

Fixes #1340, pharo-project/pharo#13907 and #1493.

  • In order to fix losing items, instead of remembering original sourceItems then trying to re-calculate from these after every move, use actual items in both lists
  • Merged duplicite parts of code for moving items
  • In order to fix filtering after moves: If the filter would result in empty list, clear the filter. if the filter would result in at least some shown items, reapply the filter. Always match filter field text with actually applied filter.
  • Moved sorting responsibility to ListPresenters (use SpAbstractListPresenter sortingBlock) instead of Chooser replacing lists items after sorting "manually" (the chooser now only provides sortingBlock during initialization)

Copy link
Member

@estebanlm estebanlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's wait for what Cyril has to say, otherwise it looks ok, I will approve preemptively :P

#package : 'Spec2-CommonWidgets-Tests'
#category : 'Spec2-CommonWidgets-Tests-Core',
#package : 'Spec2-CommonWidgets-Tests',
#tag : 'Core'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convention is actually put tag before package (because then the template will work correctly, otherwise it doesn't). I do not think this is an issue in monticello, however... maybe @jecisc knows ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the tag after the package in the Tonel format because I did not really think about the order in other places :(

So this is not related to this PR

@jecisc jecisc merged commit 63ed652 into pharo-spec:Pharo12 Dec 19, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpChooserPresenter swallows items after removing them and then adding others
3 participants