Move show password checkbox under password field

[sanjaysiddhanti]

@di

This PR implements #6722.

Screenshot before:

Screenshot after:

Let me know if you'd like any additional changes.

Fixes #6722.

[sanjaysiddhanti]

I could also move "show password" down to be horizontally aligned with "Log in" and "forgot password." I wasn't sure where exactly you wanted it. This does meet the ticket's request of making password one tab away from username.

[di]

I could also move "show password" down to be horizontally aligned with "Log in" and "forgot password."

Yes, please do that! I think that can be done with the split-layout class. We should also remove the split-layout class where it's no longer being used, like #6722 (comment) describes.

[sanjaysiddhanti]

Ok, I removed the split-layout where it is no longer used. When I apply the rest of the diff from #6722 (comment), it basically looks like my screenshot above, without horizontal alignment.

How does d845979 look? I did the following:

• Moved "show password" into the form group that has the log in and forgot password buttons
• This broke the "show password" functionality. Looks like clicking on "show password" triggers some JS code, but it needs to be in a block that connects it to the Password Controller (and be in the same block as the target element?). So, I moved the entire form group into the <div data-controller="password" class="form-group"> block. Is there a better solution?

New screenshot below

[sanjaysiddhanti]

will cause them to both be moved to the right side, but stacked on top of each other

Ah, I didn't realize that was the goal. Your suggested diff doesn't quite do it, since label and span are both inline (see below).

But when I wrap the label in a div in b98f393, I think it looks right

[sanjaysiddhanti]

@nlhkabu No rush, but just a friendly ping on this one in case it got lost.

[nlhkabu]

Hi @sanjaysiddhanti thank you for your pull request. I think it's beginning to feel a bit crowded down at the bottom right. Maybe a layout like this would be better:

You can use split-layout to position the button and checkbox and margin helpers to ensure the spacing between elements is correct.

@di I am going to remove myself as a reviewer here (lack of bandwidth) - please merge once @sanjaysiddhanti has addressed my feedback and you are happy to go ahead. Thanks 🙏

[sanjaysiddhanti]

removed split-layout--table, as it causes elements to be vertically centered in the container, whereas Nicole's screenshot shows "Show password" aligned to the top of the div

[sanjaysiddhanti]

Thanks for the feedback! Here is an updated version:

The only difference I see is that Nicole's version has double the margin between "Log in" and "Show password." I tried to achieve this with the following, but the margins were getting collapsed with the sibling element:

            <span class="display-block margin-top--large">
              <a href="{{request.route_url('accounts.request-password-reset')}}">{% trans %}Forgot password?{% endtrans %}</a>
            </span>

I also tried wrapping the span in a div class="margin-top--large"

@di did you have an idea of how to get this extra margin and does it matter to you?

[patelneel55]

Since there's now a new feature request regarding a "Keep me signed in" button at #8033, would it be beneficial to switch to a 👁️ icon, as mentioned by @bittner at #6722, to prevent a clutter of text fields under the password field?

Any thoughts? @di @sanjaysiddhanti

[di]

I think this is good as-is!

[bittner]

Nice! This change fully addresses the usability issue that was there before. Adding an 👁️ icon now only will be a cosmetic enhancement. This can certainly be done "any time". 👍

Thanks for this important improvement! 🥇

[bittner]

Nice, this change is already rolled out. Is the pipeline that does this public? It doesn't seem to be the GitHub Actions of this repository. 🤔

[brainwane]

Hi @sanjaysiddhanti! Congrats on landing your first pull request in PyPA projects. Could I ask you to take a look at #5222 and comment there if you can work on it?