Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the provider dashboard to improve the onboarding experience and filtering logic. It introduces utility functions to manage the 'configured only' filter and the visibility of the first-provider onboarding hint, which is now displayed as a conditional card rather than a full-page state. Corresponding unit tests were added to validate these utilities. Feedback was provided regarding the 'Show Configured Only' toggle, noting that tying its visual state to the effective filter value (which is forced to false when no providers exist) could lead to an inconsistent user experience when the first provider is connected.
| checked={effectiveShowConfiguredOnly} | ||
| onChange={setShowConfiguredOnly} | ||
| label={t("showConfiguredOnly")} | ||
| disabled={connections.length === 0} |
There was a problem hiding this comment.
The Toggle component's checked state is tied to effectiveShowConfiguredOnly, which is forced to false when connections.length === 0. While this correctly reflects that the filter is inactive, it creates a slightly jarring UX where the toggle might suddenly flip to 'checked' and the list of providers might shrink as soon as the user adds their first provider (if their stored preference was true).
Consider if it would be better to keep checked={showConfiguredOnly} so the toggle reflects the user's intent, even if the filter is not currently 'effective' due to the empty state logic. However, since the toggle is disabled when empty, the current implementation is also acceptable as it accurately represents the active filtering state.
|
Thank you @slider23 for this fix! 🙏 Your change to resolve the providers page empty-state deadlock has been integrated into the release/v3.8.0 branch via cherry-pick. What was applied:
Note: Because the PR was opened against |
|
Thank you so much for this contribution @slider23! The changes were highly valuable and have been manually integrated (cherry-picked) into our upcoming v3.8.0 release branch. Since there were merge conflicts that we resolved directly during the release staging, we are closing this PR. However, rest assured that your exact implementation is included and you have been officially credited in our |
Today I tried installing OmniRoute for the first time. I used the Docker image diegosouzapw/omniroute:latest. I found a bug in the UI on the page - it’s impossible to add any providers.
Code analysis revealed a deadlock on the page - if no providers have been created, the provider block isn’t displayed. I’ve made a fix and am proposing it for merge.