-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix #20475 improve labels in Deck dialogs #20492
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,53 +94,52 @@ class CreateDeckDialog( | |
| .Builder(context) | ||
| .show { | ||
| title(title) | ||
| // Resource ID for the dialog's positive action button text. | ||
| // Uses "Rename" for rename deck dialogs and "Create" for all other deck-related dialogs. | ||
| val positiveButtonTextRes = | ||
| when (deckDialogType) { | ||
| DeckDialogType.RENAME_DECK -> R.string.rename | ||
|
|
||
| DeckDialogType.DECK, | ||
| DeckDialogType.SUB_DECK, | ||
| DeckDialogType.FILTERED_DECK, | ||
| -> R.string.dialog_positive_create | ||
| } | ||
| positiveButton(positiveButtonTextRes) { | ||
| onPositiveButtonClicked() | ||
| } | ||
| positiveButton(positiveButtonTextRes) { onPositiveButtonClicked() } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated change |
||
| negativeButton(R.string.dialog_cancel) | ||
| setView(R.layout.dialog_generic_text_input) | ||
| }.input(prefill = initialDeckName, displayKeyboard = true, waitForPositiveButton = false) { dialog, text -> | ||
|
|
||
| // defining the action of done button in ImeKeyBoard and enter button in physical keyBoard | ||
| // --- Dynamic Hint for 3 different dialogs --- | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this |
||
| val inputLayout = dialog.getInputTextLayout() | ||
| when (deckDialogType) { | ||
| DeckDialogType.DECK -> inputLayout.hint = context.getString(R.string.deck_name) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the hint is set inside the .input { } callback, which runs on text updates |
||
| DeckDialogType.SUB_DECK -> inputLayout.hint = context.getString(R.string.sub_deck_name) | ||
| DeckDialogType.RENAME_DECK -> inputLayout.hint = context.getString(R.string.new_deck_name) | ||
| DeckDialogType.FILTERED_DECK -> inputLayout.hint = context.getString(R.string.deck_name) | ||
| } | ||
| // --- End Dynamic Hint --- | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this |
||
|
|
||
| val inputField = dialog.getInputField() | ||
| inputField.setOnEditorActionListener { _, actionId, event -> | ||
| if (actionId == EditorInfo.IME_ACTION_DONE || event?.keyCode == KeyEvent.KEYCODE_ENTER) { | ||
| when { | ||
| dialog.positiveButton.isEnabled -> { | ||
| onPositiveButtonClicked() | ||
| } | ||
| text.isBlank() -> { | ||
| dialog.positiveButton.isEnabled -> onPositiveButtonClicked() | ||
| text.isBlank() -> | ||
| dialog.getInputTextLayout().showSnackbar( | ||
| context.getString(R.string.empty_deck_name), | ||
| Snackbar.LENGTH_SHORT, | ||
| ) | ||
| } | ||
| else -> { | ||
| else -> | ||
| dialog.getInputTextLayout().showSnackbar( | ||
| context.getString(R.string.deck_already_exists), | ||
| Snackbar.LENGTH_SHORT, | ||
| ) | ||
| } | ||
| } | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| // we need the fully-qualified name for subdecks | ||
|
|
||
| val maybeDeckName = fullyQualifyDeckName(dialogText = text) | ||
| // if the name is empty, it seems distracting to show an error | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May I know why you removed this comment ? |
||
| if (maybeDeckName == null || !Decks.isValidDeckName(maybeDeckName)) { | ||
| dialog.positiveButton.isEnabled = false | ||
| return@input | ||
|
|
@@ -153,14 +152,8 @@ class CreateDeckDialog( | |
| dialog.getInputTextLayout().error = null | ||
| dialog.positiveButton.isEnabled = true | ||
|
|
||
| // Users expect the ordering [1, 2, 10], but get [1, 10, 2] | ||
| // To fix: they need [01, 02, 10]. Show a hint to help them | ||
| dialog.getInputTextLayout().helperText = | ||
| if (text.containsNumberLargerThanNine()) { | ||
| context.getString(R.string.create_deck_numeric_hint) | ||
| } else { | ||
| null | ||
| } | ||
| if (text.containsNumberLargerThanNine()) context.getString(R.string.create_deck_numeric_hint) else null | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated change |
||
| } | ||
| shownDialog = dialog | ||
| return dialog | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,8 @@ | |
| --> | ||
|
|
||
|
|
||
| <com.google.android.material.textfield.TextInputLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
| <com.google.android.material.textfield.TextInputLayout | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated change |
||
| xmlns:android="http://schemas.android.com/apk/res/android" | ||
| xmlns:app="http://schemas.android.com/apk/res-auto" | ||
| android:id="@+id/dialog_text_input_layout" | ||
| android:imeOptions="actionDone" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,9 @@ | |
| <string name="CardEditorCards">Cards: %1$s</string> | ||
| <string name="edit_occlusions">Edit Occlusions</string> | ||
| <string name="paste_from_clipboard" maxLength="28">Paste from clipboard</string> | ||
|
|
||
| <string name="deck_name">Deck name</string> | ||
| <string name="sub_deck_name">Subdeck name</string> | ||
| <string name="new_deck_name">New Deck name</string> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks inconsistent with sentence case |
||
| <string name="tag_name">Tag name</string> | ||
| <string name="add_new_filter_tags">Add/filter tags</string> | ||
| <string name="add_tag" maxLength="28">Add tag</string> | ||
|
|
||
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.
copy/paste: May I know why you removed this comment ?