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

Separate disabled state from non-editable state for DaxTextInput #5265

Conversation

CDRussell
Copy link
Member

@CDRussell CDRussell commented Nov 12, 2024

Task/Issue URL: https://app.asana.com/0/488551667048375/1208740714999313/f

Description

Separates non-editable state from disabled state of DaxTextInput. An input can be:

  • non-editable but enabled (i.e., read-only text that isn't greyed out, with buttons in the view still working)
  • disabled (i.e., non-editable and greyed out, and all buttons disabled too)

Steps to test this PR

Passwords

  • Go to Passwords and tap ➕ to manually start adding a password
  • Verify the input fields behave correctly
  • Save the password; verify the fields correctly go to read-only mode (non-editable state), and that the buttons on the fields work (like copy password, reveal password etc...)
  • Edit the password again and verify the fields correctly go to editable state

App launch setting

  • Go to Settings -> General -> Show on App Launch
  • Verify the URL is greyed out and non-editable when it is not selected
  • Select it, and verify the URL is no longer greyed out, and is now editable
  • Unselect it again and make sure that works correctly

Anything else that might be affected?

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @CDRussell and the rest of your teammates on Graphite Graphite

@@ -110,7 +110,7 @@ class ShowOnAppLaunchActivity : DuckDuckGoActivity() {
uncheckNewTabCheckListItem()
with(binding) {
specificPageCheckListItem.setChecked(true)
specificPageUrlInput.isEditable = true
specificPageUrlInput.isEnabled = true
Copy link
Member Author

Choose a reason for hiding this comment

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

isEnabled implies non-editability, plus you also get the disabled look

@CDRussell CDRussell marked this pull request as ready for review November 12, 2024 11:41
@CDRussell CDRussell requested review from mikescamell and malmstein and removed request for malmstein and nalcalag November 12, 2024 11:41
Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

If we want to use the disabled state, it should also work when the value is set in the XML directly. At the moment it only works if the value is set explicitly by code.
Screenshot_20241113_115901

Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

I’ve added the attribute so it can read from inflation. Rest looks good!

@CDRussell
Copy link
Member Author

I’ve added the attribute so it can read from inflation. Rest looks good!

Awesome, thanks @malmstein

@CDRussell CDRussell enabled auto-merge (squash) November 13, 2024 11:14
@CDRussell CDRussell merged commit 4e3e1cc into develop Nov 13, 2024
7 checks passed
@CDRussell CDRussell deleted the feature/craig/fix/craig/daxtextinput_should_allow_noneditable_without_disabling branch November 13, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants