Skip to content

Ability to unset emptyStateText in Autocomplete.Menu #4002

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

Conversation

peterbe
Copy link
Contributor

@peterbe peterbe commented Dec 1, 2023

Closes #3966

Changelog

When you set emptyStateText={false} or emptyStateText={null} it disables the default display which normally shows when there is allItemsToRender.length === 0.

New

Changed

(Not sure what to fill in here)

Removed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan

Testing & Reviewing

I'm actually not familiar with how to test things with Storybook.

I've been using https://github.com/peterbe/primer-autocomplete to play.
I run npm run mock-server in one terminal, and in the other I run:

./build-and-install-primer-react.sh && npm run dev

Now I can manually test my active changes in /Users/peterbe/dev/GITHUB/primer/react

Result:

Screen.Recording.2023-12-01.at.4.36.29.PM.mov

(What's not obvious in the video is that a second after I've typed the y in "holly" the next thing is that hit the Enter key, which (Nextjs) redirects to the full site-search)

What this implementation does is that it entirely disables the empty-state text by setting:

emptyStateText={false}

So when the cursor is put into the input field, before any XHR can fetch anything, there are no items to suggest. So nothing happens and nothing is shown just because the cursor is put into the input.
Then when you type and, after XHR loading, there's exactly 0 found suggestions, only then does it add a faux item which I watch out for in the onSubmit.

See: https://github.com/peterbe/primer-autocomplete/blob/4af3e861a5fe8443626ec4899d1aa3f29b05b9e7/src/pages/index.tsx#L218
and https://github.com/peterbe/primer-autocomplete/blob/4af3e861a5fe8443626ec4899d1aa3f29b05b9e7/src/pages/index.tsx#L175-L180 for the "faux" suggestion.
And lastly, I complete the full search based on the typed input, these lines:
https://github.com/peterbe/primer-autocomplete/blob/4af3e861a5fe8443626ec4899d1aa3f29b05b9e7/src/pages/index.tsx#L224-L231

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

Copy link

changeset-bot bot commented Dec 1, 2023

🦋 Changeset detected

Latest commit: 989433e

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

This PR includes changesets to release 1 package
Name Type
@primer/react 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

@peterbe
Copy link
Contributor Author

peterbe commented Dec 1, 2023

@mperrotti (CC @siddharthkp )
Be gentle! My first PR against primer. I'd love some guidance on the paperwork around writing changesets and/or how to update Storybook to examplify this new possibility.
I'm quite fond of the net result now. It seems to be "exactly" what was concluded in the Primer A11y office hours meeting we had.

@lesliecdubs
Copy link
Member

Hey @peterbe, thanks for the contribution ✨ ! If you're ready for review, would you please mark this draft as ready and the reviewers will get it into their queue? Thanks!

@peterbe peterbe marked this pull request as ready for review December 6, 2023 14:59
@peterbe peterbe requested review from a team and joshblack December 6, 2023 14:59
@peterbe
Copy link
Contributor Author

peterbe commented Dec 11, 2023

@mperrotti I see that you're assigned to the issue
Perhaps you can lend a helping hand how to move this PR forward. I'd unaccustomed to the Storybook side and the changeset paperwork is new to me.

@mperrotti
Copy link
Contributor

Sorry for the delay @peterbe - taking a look now.

@peterbe
Copy link
Contributor Author

peterbe commented Dec 18, 2023

I don't understand the failing CI checks. Is that anything I should heed?

@mperrotti
Copy link
Contributor

We just merged a fix for those. Let me try re-running...

@mperrotti
Copy link
Contributor

@peterbe - try updating your fork to pull the latest main branch

…u' of github.com:peterbe/primer-react into 3966-ability-to-unset-emptystatetext-in-autocompletemenu
@peterbe
Copy link
Contributor Author

peterbe commented Dec 18, 2023

@peterbe - try updating your fork to pull the latest main branch

Did that. Same error.

@peterbe
Copy link
Contributor Author

peterbe commented Dec 18, 2023

I think it's because https://github.com/primer/react/blob/1116586269f6fadf9127661f8aef3255b447e8a1/.github/workflows/deploy_preview.yml doesn't specify a permissions: at the top of the file.

@mperrotti
Copy link
Contributor

@peterbe this should fix the issues once it's merged: #4087

@joshblack joshblack removed their request for review December 19, 2023 23:20
@mperrotti mperrotti added this pull request to the merge queue Dec 21, 2023
Merged via the queue into primer:main with commit 1a1d89c Dec 21, 2023
@primer primer bot mentioned this pull request Dec 21, 2023
@peterbe peterbe deleted the 3966-ability-to-unset-emptystatetext-in-autocompletemenu branch December 21, 2023 21:30
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.

Not possible to show nothing if no items match in Autocomplete
3 participants