Skip to content
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

Don't allow empty name in flag #17856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gayatriii0803
Copy link
Contributor

@gayatriii0803 gayatriii0803 commented Jan 22, 2025

Purpose / Description

Fixes the issue where a flag could be renamed to an empty value in the Card Browser. Now, empty flag names are not allowed.

Fixes

Approach

If user tries to save empty flag name then set it back to the default name which was used.

How Has This Been Tested?

Physical device (OPPO F21 Pro 5G)

flag.mp4

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@gayatriii0803 gayatriii0803 force-pushed the flag branch 2 times, most recently from 440f0e0 to e691ed8 Compare January 22, 2025 12:50
@@ -92,6 +93,10 @@ class FlagAdapter(

holder.saveButton.setOnClickListener {
val updatedTextName = holder.flagNameEdit.text.toString()
if (updatedTextName.isEmpty()) {
showThemedToast(holder.itemView.context, R.string.empty_flag_message, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

No toasts please, the ideal approach would be to utilise the TextInputLayout and show error text or do it the Anki way i.e. if user tries to save empty flag name then set it back to the default name which was used last

@lukstbit lukstbit added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author labels Jan 31, 2025
Copy link
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Second Approval Has one approval, one more approval to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag name should not be empty
3 participants