-
-
Notifications
You must be signed in to change notification settings - Fork 734
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
feat: Improve custom API URL dialog #2033
Conversation
@Axelen123 Is #1973 related? The issue appears to suggest replacing the pref value to actual description in the Settings page for API URL listview's description. I could tackle that here since it's (soon-to-be:tm:) related to this PR. |
Sure, you can do it in this PR |
Oh, it looks like you are right. You can unlink the issue or take it on in this PR |
Co-authored-by: oSumAtrIX <[email protected]>
Snapshot of a822c25 |
Co-authored-by: Ushie <[email protected]>
@Preview | ||
@Composable | ||
private fun APIUrlDialogPreview() { | ||
APIUrlDialog("https://example.com") {} |
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.
Should this not be the default API by default?
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.
Any value is fine here since this is only shown to developer using Android Studio or any IDE that support rendering a composable.
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.
I guess using default api makes more sense then
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.
This section is only to preview the design of the composable, it can be removed from this PR with no consequence
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.
I am not talking specifically about the section but the text shown in the section. So then either removing or using the default api here works
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.
Looking good apart from the one review
Script Execution UTC Time: null Signed-off-by: validcube <[email protected]>
@ https://canary.discord.com/channels/952946952348270622/952987428786941952/1258456626457739415
Note
This PR include three changes
Fix