-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat: show enabled provider only #10933
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
base: v2
Are you sure you want to change the base?
Conversation
- Add hide disabled providers toggle in provider settings - Update preference schema to store the setting - Refactor provider list layout to accommodate new toggle
| 'app.proxy.mode': PreferenceTypes.ProxyMode | ||
| // redux/settings/proxyUrl | ||
| 'app.proxy.url': string | ||
| 'app.settings.provider.hide_disabled': boolean |
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.
Note
This comment was translated by Claude.
Preference already contains setting. This naming is semantically redundant.
Additionally, the app prefix refers to globally effective and application-wide related configurations, so it shouldn't be used here.
The correct naming should be ui.provider.show_disabled.
- This is a UI category option related to providers.
- We generally conventionally use affirmative rather than negative expressions for options (so use show instead of hide, and the default value needs to be adjusted accordingly).
Original Content
preference就是包含setting。这个命名语义重复了
此外,app前缀是指全局生效和应用整体相关的配置,这里不应该用app
正确应该是 ui.provider.show_disabled
- 这是一个ui类别的,和provider相关的选项
- 我们一般约定用正义而不是反义来表达选项(所以用show而不是hide,对应默认值需要调整)
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.
Done
…roviders Update the provider settings to use a more intuitive "show disabled" toggle instead of "hide disabled". This includes updating the preference schema, default value, and UI components to reflect this change.
…/cherry-studio into feat/show-enabled-provider-only
|
Note This issue/comment/review was translated by Claude.
That's not a bad idea either. However, the notion of enabling all providers by default isn't ideal, as it would force new users to individually disable the providers they don't need. Building on this idea, we could divide the entire list into two groups: enabled and disabled, with the disabled group collapsed by default. Moreover, this solution should not require adding new data, so there's no need to wait for v2; it can be quickly merged into main. Original Content
That's not a bad idea either. However, the notion of enabling all providers by default isn't ideal, as it would force new users to individually disable the providers they don't need. Building on this idea, we could divide the entire list into two groups: enabled and disabled, with the disabled group collapsed by default. Moreover, this solution should not require adding new data, so there's no need to wait for v2; it can be quickly merged into main. |

What this PR does
Fixes #10885
Why we need it and why it was done in this way
The following tradeoffs were made:
The following alternatives were considered:
Links to places where the discussion took place:
Breaking changes
If this PR introduces breaking changes, please describe the changes and the impact on users.
Special notes for your reviewer
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
Release note