-
Notifications
You must be signed in to change notification settings - Fork 583
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
Select Panel: Add built-in "No matches" item #4920
Conversation
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/341768 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @siddharthkp, thanks for working on this 😄 However I'm not sure I understand the circumstances for using a "No matches" item. Is there a specific use-case or mockup you're working from?
EDIT: I just looked at the linked issue showing how teams are getting around the lack of a "no items" mechanism. I personally don't think we should provide both a "No matches" item and an empty state as described in https://github.com/github/primer/issues/3922. Since the empty state is officially part of the mockups design provided, I think we should avoid supporting an official "No matches" item and encourage teams to migrate.
I agree with you. I was under the impression https://github.com/github/primer/issues/3922 was not part of the current epic and we would only work on it next quarter with https://github.com/github/primer/issues/3399:) Happy to update the designs
I'm not sure if I agree on the last bit about encourage teams to migrate. It would help us to have a default no-results state that matches the new design with the option to customise the message. Update: On second thoughts, given how loading states (https://github.com/github/primer/issues/3921) is very similar design wise. I think we should address this after merging that. Closing this PR because there isn't much in this PR that would carry through |
Ah yeah, I think this work was originally scoped under #3399 but I moved it into the group of issues due 9/27 because it's related pretty directly to accessibility.
The good news is that "No matches" items will continue to work, so teams can either migrate and reap the accessibility rewards or choose to stick with what they already have and maintain existing functionality.
Yes! That was my thinking as well. |
primer_react_select_panel_with_modern_action_list
)Rollout strategy
Testing & Reviewing
Merge checklist