1185780 - disable save if you delete the entire password in about:logins

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Logins, Passwords and Form Fill, defect)

Comment 1

[Barbara Bermes [:barbara] - NI please!]

As discussed with ally, we'll go ahead and include this into the first mvp for passwords.

I suggest to grey out the button to represent "disable".

Anthony, is that good with you?

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Sounds reasonable. 

Ally, do you already have a disabled grey in the palette from the Accounts team? I see from my original design files it was spec'd at #C0C9DD but I'm not sure if they changed it.

If not, our "disabled grey" in our palette works (#BFBFBF)

Comment 3

[Allison Naaktgeboren :ally]

Attachment: aboutLoginsDisableSaveOnEmptyPwd

The default behavior of a disabled button according to our base css is to disappear the button (visibility:none iirc). This is the behavior of the patch above. 

It occurs to me you might prefer the button to remain visible but a gray color to indicate that it can't be pushed. What would you like the disabled update button to look like?

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Exactly the same, just with the grey provided above :) 

Thanks Ally!

Comment 5

[Allison Naaktgeboren :ally]

thanks

Comment 6

[Allison Naaktgeboren :ally]

Attachment: tmp_11933-Screenshot_2015-07-24-15-20-371777088428.png

Comment 7

[Allison Naaktgeboren :ally]

Attachment: aboutLoginsDisableSaveOnEmptyPwd

Comment 8

[Chenxia Liu [:liuche]]

Comment on attachment 8638806 [details] [diff] [review]
aboutLoginsDisableSaveOnEmptyPwd

Review of attachment 8638806 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry this took so long! Thanks ally, this works great.

::: mobile/android/chrome/content/aboutLogins.js
@@ +210,5 @@
> +
> +      if (newPassword === "") {
> +        updateBtn.disabled = true;
> +        updateBtn.classList.add("disabled-btn");
> +      } else if ((newPassword != "") && (updateBtn.disabled === true)) {

For correctness, shouldn't the newPassword comparison use a !== as well?

::: mobile/android/chrome/content/aboutLogins.xhtml
@@ +65,5 @@
>          <input type="password" id="password" name="password" value="password" class="edit-login-input" />
>          <button id="password-btn"></button>
>        </div>
>        <div class="edit-login-div">
> +        <button id="update-btn" class="update-btn">&aboutLogins.update;</button>

Is it good form to have the id and class be the same?

::: mobile/android/themes/core/aboutLogins.css
@@ +107,5 @@
>    border-width: 0px;
>    margin-top: 10px;
>  }
> +.disabled-btn {
> +  background-color: #BFBFBF;

Can you add a comment here, referencing this as disabled_grey from colors.xml in our Java code? You can match what you did above.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/b4f95c2cc243

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b4f95c2cc243