Skip to content

[906] chore: clear search results when focused outside the Search widget#909

Merged
justEhmadSaeed merged 4 commits intomainfrom
906-clear-search-widget-results-with-input
Nov 21, 2024
Merged

[906] chore: clear search results when focused outside the Search widget#909
justEhmadSaeed merged 4 commits intomainfrom
906-clear-search-widget-results-with-input

Conversation

@justEhmadSaeed
Copy link
Copy Markdown
Collaborator

@justEhmadSaeed justEhmadSaeed commented Nov 18, 2024

Describe your changes

#906

Screenshots [Optional]

Screen.Recording.2024-11-20.at.5.10.23.PM.mov

Issue ticket number and link

Closes #

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added tests
  • I have added a changeset pnpm changeset add
  • I have added example usage in the kitchen sink app

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Nov 18, 2024

🦋 Changeset detected

Latest commit: 76150bb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@ensembleui/react-runtime Patch
@ensembleui/react-kitchen-sink Patch
@ensembleui/react-preview Patch
@ensembleui/react-starter Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 18, 2024

Visit the preview URL for this PR (updated for commit 76150bb):

https://react-kitchen-sink-dev--pr909-906-clear-search-wid-72atvv6i.web.app

(expires Wed, 27 Nov 2024 20:57:07 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 6267897ade2ba783b6db70a53a60fc3946d625e9

Comment thread packages/runtime/src/widgets/Search.tsx Outdated
renderLabel(label, labelValue)
}
notFoundContent={notFoundContentRenderer}
onBlur={handleClear}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't we expose the onBlur property with EnsembleAction instead of clearing the results.

Clearing means user will lost the previously searched results, this might solve the current problem, but not a general use case

CC: @evshi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The issue we are trying to solve here is if the value is emptied, then the search results should also be emptied. It doesn't necessarily have anything to do with blur. Ticket for context https://atlashealth.atlassian.net/browse/TRINITY-1461

@justEhmadSaeed what is clearing the search input value?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@evshi setSearchValue(null) is clearing the input value as that's the default behavior of the Select comp. of antd

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In that case, we should add an effect/fix the effect that syncs the search results with the value. If the value was set to be null, the search results should get cleared @justEhmadSaeed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@evshi updated the PR

Comment thread packages/runtime/src/widgets/Search.tsx Outdated
Comment on lines +77 to +81
useEffect(() => {
if (!searchValue) {
setHasCleared(true);
}
}, [searchValue]);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we need hasCleared? I think if you use isEmpty instead of isNil and isNull then we can get rid of the extra flag.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@evshi yeah if we modify the condition to return an empty array if either searchValue is empty or hasCleared is true

@justEhmadSaeed justEhmadSaeed merged commit cc58c97 into main Nov 21, 2024
@justEhmadSaeed justEhmadSaeed deleted the 906-clear-search-widget-results-with-input branch November 21, 2024 05:27
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.

3 participants