[SelectPanel] Filter input border and loading/error panel min-height

[jamieshark]

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

This PR addresses a few design items towards the SelectPanel component (https://github.com/github/primer/issues/3875). Namely, it adds a divider below the filter input (if present), it moves the filter banner error to be above the input, and adds a min-height that corresponds to the overlay size for the loading / error states.

Screenshots

Integration

List the issues that this change affects.

• Closes https://github.com/github/primer/issues/3879
• Closes https://github.com/github/primer/issues/3881
• Closes https://github.com/github/primer/issues/3880
• Closes https://github.com/github/primer/issues/3934#issue-2514579172

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

Anything you want to highlight for special attention from reviewers?

Accessibility

• Fixes axe scan violation - This change fixes an existing axe scan violation.
• No new axe scan violation - This change does not introduce any new axe scan violations.
• New axe violation - This change introduces a new axe scan violation. Please describe why the violation cannot be resolved below.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Lookbook)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

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

[changeset-bot]

🦋 Changeset detected

Latest commit: 9eb76f1

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

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	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]

Uh oh! @jamieshark, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[github-actions]

Uh oh! @jamieshark, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[github-actions]

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

[camertron]

Nice! I'll let @langermank give the final go-ahead tho 😄

[maximedegreve]

Noticed a few small things. Some might be able to be addressed in this issue others might need opening of new issues.

1. The margin above the title should only be 8px and not 16px.

2. The example below closes the panel when I initiate a filter query and press enter. Since the focus is the input and not on an item this is unexpected and incorrect. The cursor is also in the wrong spot after opening it again. I can't remember 100% sure anymore but I think it should wipe the filter value when closing the panel.

CleanShot.2024-09-09.at.10.22.10.mp4

3. Multi select should have visual checkboxes (not semantically) to visually indicate to users it won't close the panel when making a multiple selection. Single select is correct to use the checkmark but multi select isn't. See the screenshots in our docs.

4. The empty state is also missing a min-height similar to the error/loading states

5. The playground example changes in width after results are loaded in. This causes a layout swift we want to avoid.

layoutshift.mp4

[jamieshark]

@maximedegreve

1. I have addressed this with my recent changes, though I would like to note that the spec specified 14px and I'm wondering what can be done to mitigate this confusion in the future.
2. I have created another issue for this: https://github.com/github/primer/issues/3932
3. I have created another issue for this: https://github.com/github/primer/issues/3933
4. I have created an issue for this (https://github.com/github/primer/issues/3934) and have addressed this in my recent changes.
5. I believe this is happening because the overlay size is set to :auto. If you would like to avoid width changes, it might help to select a different size. Otherwise, there's not really any way for us to know the exact width of the content that's coming in.

[maximedegreve]

Approving since the other reported issues will be fixed as separate issues.

[kendallgassner]

✅ For single-select, I tested that if a page limit is added the selected state is updated correctly as Items are loaded/unloaded.

[kendallgassner]

🔴 I did find one niche bug. If a selectedItem does not initial show due to a page limit... It is never selected. See video:

[kendallgassner]

💟 ✅

[camertron]

Nice!