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

Remove fast table search by default #1553

Open
wants to merge 1 commit into
base: Pharo13
Choose a base branch
from
Open

Remove fast table search by default #1553

wants to merge 1 commit into from

Conversation

jecisc
Copy link
Member

@jecisc jecisc commented Jun 4, 2024

It often happens is a table or list has the focus that we start to type and a search popover appears but it filters nothing. And in multiple UIs, we have a filter done with an input field under the list and both are overlapping and it becomes really confusing.

I propose to disable the FastTable search by default and I also remove a dead method

It often happens is a table or list has the focus that we start to type and a search popover appears but it filters nothing. And in multiple UIs, we have a filter done with an input field under the list and both are overlapping and it becomes really confusing.

I propose to disable the FastTable search by default and I also remove a dead method
@Ducasse
Copy link
Contributor

Ducasse commented Jun 13, 2024

@jecisc could we rename the method disableFunction into disableSearch?
Because disableFunction is kind of obscure, don't think?

@jecisc
Copy link
Member Author

jecisc commented Jun 13, 2024

It is call function because it can be more than a search. But the search function is the one enabled by default

@Ducasse
Copy link
Contributor

Ducasse commented Jun 13, 2024

Looks like a not so good name to me.

@Ducasse
Copy link
Contributor

Ducasse commented Jun 13, 2024

/home/runner/.smalltalkCI/_builds/TravisCI.image
Pharo13.0.0SNAPSHOT
Build information: Pharo-13.0.0+SNAPSHOT.build.87.sha.0ac6afe9a01de2cd856af00b775bf8b48d7a329a (64 Bit)


 > Done in 1.000ms.
Loading project...
Syntax Error on line 2: 'Undeclared variable'
=============================================
1: #( 'BaselineOfSpec2' 'BaselineOfSpecCore' ) do: [ :each | 
2: 	(RPackageOrganizer default packageNamed: each ifAbsent: [ nil ]) 
    _^_
3: 		ifNotNil: [ :aPackage | aPackage removeFromSystem ] ]
CodeError:OCUndeclaredVariableNotice UndefinedObject>>#DoIt 62:Undeclared variable->RPackageOrganizer
OCUndeclaredVariableNotice(RBNotice)>>signalError
OpalCompiler>>checkNotice:
[ :n |
			| check |
			check := self checkNotice: n.
			check ifNil: [ ^ ast ].
			check ifFalse: [
				ast := nil.

@Ducasse Ducasse closed this Jun 13, 2024
@Ducasse Ducasse reopened this Jun 13, 2024
@Ducasse
Copy link
Contributor

Ducasse commented Jun 13, 2024


SpListSearchTest
 ✗ #testSearchWithFunction (78ms)
 ✗ #testTypePerformsSearch (43ms)
 ✗ #testSearchWithFunction (39ms)
 ✗ #testTypePerformsSearch (30ms)

SpTableCommonPropertiestTest
 ✗ #testTypePerformsSearch (38ms)
 ✗ #testTypePerformsSearch (41ms)

SpTableSearchTest
 ✗ #testSearchWithFunction (54ms)
 ✗ #testTypePerformsSearch (26ms)
 ✗ #testSearchWithFunction (52ms)
 ✗ #testTypePerformsSearch (36ms)

SpListCommonPropertiestTest
 ✗ #testTypePerformsSearch (55ms)
 ✗ #testTypePerformsSearch (55ms)

@demarey
Copy link
Collaborator

demarey commented Jun 13, 2024

Looks like a not so good name to me.

+1

@jecisc
Copy link
Member Author

jecisc commented Jun 13, 2024

Maybe we could update the name in FastTable, but here I am updating a usage of FastTable.
FastTable is in Morphic so I cannot update it in this PR

@Ducasse
Copy link
Contributor

Ducasse commented Jun 15, 2024

Sure we should do that in another PR.

@Ducasse
Copy link
Contributor

Ducasse commented Jun 25, 2024

ReleaseTest
 ✗ #testUndeclared (151ms)


  Executed 131 Tests with 1 Failures and 0 Errors in 61.36s.


To reproduce the failed build locally, download smalltalkCI, and try to run something like:

@Ducasse
Copy link
Contributor

Ducasse commented Jun 25, 2024

I'm confused

###########
# Summary #
###########

SpListSearchTest
 ✗ #testSearchWithFunction (78ms)
 ✗ #testTypePerformsSearch (43ms)
 ✗ #testSearchWithFunction (39ms)
 ✗ #testTypePerformsSearch (30ms)

SpTableCommonPropertiestTest
 ✗ #testTypePerformsSearch (38ms)
 ✗ #testTypePerformsSearch (41ms)

SpTableSearchTest
 ✗ #testSearchWithFunction (54ms)
 ✗ #testTypePerformsSearch (26ms)
 ✗ #testSearchWithFunction (52ms)
 ✗ #testTypePerformsSearch (36ms)

SpListCommonPropertiestTest
 ✗ #testTypePerformsSearch (55ms)
 ✗ #testTypePerformsSearch (55ms)


  Executed 1545 Tests with 12 Failures and 0 Errors in 45.56s.


To reproduce the failed build locally, download smalltalkCI, and try to run something like:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants