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

[Backport] rapid click on two different items in a list gets interpreted as a double-click on the first item #14345

Open
wants to merge 3 commits into
base: Pharo11
Choose a base branch
from

Conversation

ironirc
Copy link
Contributor

@ironirc ironirc commented Jul 21, 2023

This PR solves the problem with 2 subsequent clicks (within double click timing) on 2 different items of FTTableMorph.
Before the fix, this would execute a double click command on the first clicked item.
The fix involves comparing the 2 click positions and resulting item indexes.

A second problem occurs when double clicking on an already selected item.
Before the fix, the item gets de-selected, and nothing further happend.
This PR ensures that item is selected before dispatching the double click command.

@request-info
Copy link

request-info bot commented Jul 21, 2023

This issue has either a default title or empty body. We would appreciate it if you could provide more information. Note: I am not a very intelligent bot, I can only react to new comments. Please add a comment for me if you update the body or title.

@request-info request-info bot added the Status: more-information-needed This issue will be auto-closed if no answer in 14 days label Jul 21, 2023
@jecisc
Copy link
Member

jecisc commented Jul 21, 2023

Hi,

Current bug fixes should be done on Pharo 12. If the fix is integrated in Pharo 12 and works fine, we can then backport it to Pharo 11.
I'll try to rebase it and see if we have conflict against P12

@jecisc jecisc changed the base branch from Pharo11 to Pharo12 July 21, 2023 12:44
@jecisc jecisc changed the base branch from Pharo12 to Pharo11 July 21, 2023 12:45
@jecisc
Copy link
Member

jecisc commented Jul 21, 2023

It brings other changes so I cannot just rebase it against P11

@ironirc
Copy link
Contributor Author

ironirc commented Jul 21, 2023

@jecisc What should I do?

@jecisc
Copy link
Member

jecisc commented Jul 21, 2023

I'll try to cherry pic your fix against P12.
We can keep this PR for the backport but it should be merge after the P12 one

@jecisc jecisc changed the title Fixes https://github.com/pharo-project/pharo/issues/14168 [Backport] rapid click on two different items in a list gets interpreted as a double-click on the first item Jul 21, 2023
@jecisc
Copy link
Member

jecisc commented Jul 21, 2023

#14346

@jecisc jecisc removed the Status: more-information-needed This issue will be auto-closed if no answer in 14 days label Jul 21, 2023
@MarcusDenker MarcusDenker reopened this Sep 6, 2023
@MarcusDenker
Copy link
Member

(I did a close/reopen so we run the tests against the newest version of Pharo11)

@MarcusDenker
Copy link
Member

The fix is in Pharo12 since end of July, it seems no problems came up.

We can merge the backport when the tests look ok

@MarcusDenker
Copy link
Member

The CI run has some tests failing due to a DNU for #asPseudoDoubleClickEvent, for example #testDoubleClickActivatesRowInDoubleClickActivationMode

Error
Message not understood: MouseEvent >> #asPseudoDoubleClickEvent
Stacktrace
MessageNotUnderstood
Message not understood: MouseEvent >> #asPseudoDoubleClickEvent
MouseEvent(Object)>>doesNotUnderstand: #asPseudoDoubleClickEvent
SpFTTableMorph(FTTableMorph)>>doubleClick:
SpMorphicBackendForTest>>doubleClickFirstRowAndColumn:
SpListCommonPropertiestTest(SpAbstractListCommonPropertiestTest)>>testDoubleClickActivatesRowInDoubleClickActivationMode
SpListCommonPropertiestTest(TestCase)>>performTest
[ presenter := self classToTest new.
			self initializeTestedInstance.
			super performTest ] in SpListCommonPropertiestTest(SpAbstractAdapterTest)>>performTest
SpMorphicBackendForTest>>runTest:
SpListCommonPropertiestTest(SpAbstractAdapterTest)>>performTest



@MarcusDenker MarcusDenker added the Status: Need more work The issue is nearly ready. Waiting some last bits. label Sep 6, 2023
@jecisc
Copy link
Member

jecisc commented Sep 6, 2023

I think this is because a change was integrated to Spec for the Pharo 11 branch but we need to do a new release

@jecisc
Copy link
Member

jecisc commented Sep 6, 2023

BaselineOfPharo class>>#spec needs to be updated to point to v1.2.4 instead of v1.2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Need more work The issue is nearly ready. Waiting some last bits.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants