Skip to content

fix: Allow repo dropdown to open on input when empty#4422

Open
jlsherrill wants to merge 1 commit intoosbuild:mainfrom
jlsherrill:better_repos
Open

fix: Allow repo dropdown to open on input when empty#4422
jlsherrill wants to merge 1 commit intoosbuild:mainfrom
jlsherrill:better_repos

Conversation

@jlsherrill
Copy link
Copy Markdown
Contributor

@jlsherrill jlsherrill commented May 6, 2026

Removes inputValue condition from onInputClick so the dropdown opens when clicking the search box, even when empty, allowing users to see unfiltered repositories immediately.

Fixes HMS-10642

@jlsherrill jlsherrill requested a review from a team as a code owner May 6, 2026 19:17
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • Setting skip: false on the repositories query means it will fire even when the dropdown is closed and no search is in progress; consider conditioning skip on isOpen and/or debouncedFilterValue to avoid unnecessary network requests.
  • The new empty-state branches for filtered vs unfiltered results add more conditional complexity in the render; you might consider extracting this into a small helper function (e.g., renderEmptyState(debouncedFilterValue)) to keep the JSX easier to scan.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Setting `skip: false` on the repositories query means it will fire even when the dropdown is closed and no search is in progress; consider conditioning `skip` on `isOpen` and/or `debouncedFilterValue` to avoid unnecessary network requests.
- The new empty-state branches for filtered vs unfiltered results add more conditional complexity in the render; you might consider extracting this into a small helper function (e.g., `renderEmptyState(debouncedFilterValue)`) to keep the JSX easier to scan.

## Individual Comments

### Comment 1
<location path="src/Components/CreateImageWizard/steps/Repositories/tests/Repositories.test.tsx" line_range="93-91" />
<code_context>
       resolveSearch(JSON.stringify({ data: [], meta: { count: 0 } }));
     });

+    test('shows unfiltered repositories when dropdown opens', async () => {
+      fetchMock.mockResponse(
+        createFetchHandler({ repositories: mockRepositoryResults }),
+      );
+      renderRepositoriesStep();
+      const user = createUser();
+
+      // Open dropdown by clicking toggle
+      const toggle = await screen.findByRole('button', { name: /menu toggle/i });
+      await user.click(toggle);
+
+      // Should show repositories without typing
+      expect(
+        await screen.findByRole('option', { name: /01-test-valid-repo/i }),
+      ).toBeInTheDocument();
+      expect(
+        await screen.findByRole('option', { name: /04-test-another-valid-repo/i }),
+      ).toBeInTheDocument();
+    });
+
+    test('shows appropriate empty state message based on filter state', async () => {
</code_context>
<issue_to_address>
**issue (testing):** Add a test that covers opening the dropdown via the search input click when the input is empty.

This change fixes behavior for clicking the empty search input, but the new test exercises opening via the toggle button instead. To directly cover the regression, please add a test that:

- Renders the component with no initial filter value
- Locates the search textbox (role="textbox", name /filter repositories/i)
- Clicks the textbox while the dropdown is closed and empty
- Asserts that the dropdown opens (options or the correct empty/loading state are visible)

That will specifically validate the `onInputClick` path this PR is addressing.
</issue_to_address>

### Comment 2
<location path="src/Components/CreateImageWizard/steps/Repositories/tests/Repositories.test.tsx" line_range="113-84" />
<code_context>
+      ).toBeInTheDocument();
+    });
+
+    test('shows appropriate empty state message based on filter state', async () => {
+      fetchMock.mockResponse(
+        createFetchHandler({ repositories: [] }),
+      );
+      renderRepositoriesStep();
+      const user = createUser();
+
+      // Open dropdown - should show "No repositories available"
+      const toggle = await screen.findByRole('button', { name: /menu toggle/i });
+      await user.click(toggle);
+
+      expect(
+        await screen.findByRole('option', { name: /no repositories available/i }),
+      ).toBeInTheDocument();
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the empty‑state test by asserting that the previous message is no longer rendered after the filter changes.

In `shows appropriate empty state message based on filter state`, you already check that the message changes after typing. Please also assert that the initial "No repositories available" option is no longer present after typing (e.g. via `queryByRole` + `expect(...).not.toBeInTheDocument()`) to verify the component doesn’t render both empty states at once.

Suggested implementation:

```typescript
      expect(
        await screen.findByRole('option', { name: /no repositories match your filters/i }),
      ).toBeInTheDocument();

      // Initial empty-state message should no longer be rendered
      expect(
        screen.queryByRole('option', { name: /no repositories available/i }),
      ).not.toBeInTheDocument();

```

If the text of the second empty-state message is slightly different (e.g. different casing or wording), adjust the `/no repositories match your filters/i` regex in the SEARCH block to match the actual string used in your test.

If you used a different role or label for the initial empty-state (instead of `role: 'option'` or label `/no repositories available/i`), update the `queryByRole` arguments accordingly so the assertion targets the same element as your first empty-state check.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

).toBeInTheDocument();

resolveSearch(JSON.stringify({ data: [], meta: { count: 0 } }));
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (testing): Add a test that covers opening the dropdown via the search input click when the input is empty.

This change fixes behavior for clicking the empty search input, but the new test exercises opening via the toggle button instead. To directly cover the regression, please add a test that:

  • Renders the component with no initial filter value
  • Locates the search textbox (role="textbox", name /filter repositories/i)
  • Clicks the textbox while the dropdown is closed and empty
  • Asserts that the dropdown opens (options or the correct empty/loading state are visible)

That will specifically validate the onInputClick path this PR is addressing.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.62%. Comparing base (57abb52) to head (39ed952).

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4422      +/-   ##
==========================================
+ Coverage   74.60%   74.62%   +0.02%     
==========================================
  Files         229      229              
  Lines        7563     7563              
  Branches     2829     2832       +3     
==========================================
+ Hits         5642     5644       +2     
+ Misses       1664     1662       -2     
  Partials      257      257              
Files with missing lines Coverage Δ
...steps/Repositories/components/RepositorySearch.tsx 90.00% <100.00%> (+2.85%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57abb52...39ed952. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Removes inputValue condition from onInputClick so the dropdown
opens when clicking the search box, even when empty, allowing
users to see unfiltered repositories immediately.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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.

1 participant