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

tsify data catalog tab and convert it to functional component #7476

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

Conversation

zoran995
Copy link
Collaborator

What this PR does

This is needed for mobx-react upgrade as part of an upgrade to react 18.

Test me

How should reviewers test this? (Hint: If you want to provide a test catalog item, create a Gist of its catalog JSON, add its url and your branch name to this url: http://ci.terria.io/<branch name>/#clean&<raw url of your gist> , and then paste that in the body of this PR)

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated relevant documentation in doc/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

placeholder={searchPlaceholder}
debounceDuration={
terria.catalogReferencesLoaded &&
searchState.catalogSearchProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

This part of the condition is already checked on line 51.

interface DataCatalogTabProps {
items?: unknown[];
searchPlaceholder?: string;
onActionButtonClicked?: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

This type is (item: something) => void.


const changeSearchText = (newText: string) => {
runInAction(() => {
viewState.searchState.catalogSearchText = newText;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use searchState.X instead of viewState.searchState.X just like the return statement a few lines below?

@pjonsson
Copy link
Contributor

I did the straightforward (?) conversion to ES6 objects of some of this stuff in #7415, you might be able to cherry-pick the non-overlapping parts on top of this to get more TSification of this part of the code.

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.

2 participants