[Login] Show password in login/sign up form

[cavasinf]

Description

Fix #209

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I updated the documentation (see here)
• I agree that this code will be published under the MIT license

[cavasinf]

A new translation, Show password, has been added.
The feature has been implemented within a block instead of as an option, to simplify the structure.

[cavasinf]

Wait.. should we add at least the en trans for "Show password" or weblate will find it magically?

[kevinpapst]

Yes, good catch! We need the translation in the base language, so Weblate can pick it up.

[cavasinf]

Hey, busy weeks lately, but we have basic trans now (+ fr)

[kevinpapst]

Thanks 👍

I gave it a quick test and have one question:
To me the open eye says "I can see the password" and the strikethrough eye means "password cannot be seen".
Don't you think the eyes are in the wrong order?

[cavasinf]

To me the open eye says "I can see the password" and the strikethrough eye means "password cannot be seen".
Don't you think the eyes are in the wrong order?

I understand your vision of the button.
The button is not meant to indicate the current state of the input but rather the consequence of clicking it.

After looking at some references, here are a few examples:

• GitLab
• OpenAI
• Netflix
• CNN
• Odoo

Considering your perspective, I believe we should add a new label: "Hide password".
This emphasizes the action rather than the state.

[kevinpapst]

Ok, if these platforms have it this way, I am fine with it as well.
But regarding the translation, you are right.
Having "Show password" there, no matter which state, is misleading/wrong. Shall we remove it altogether or do you want to add a second translation?

[cavasinf]

Shall we remove it altogether or do you want to add a second translation?

I'll add the second translation, and the js will only hide/show the a element, which will be doubled with the two different titles. (no need to target the svgs anymore)

Do we keep the raw SVG or do we use tabler_icon ? I don't really know what to do because that means we are forcing devs to use fontAwesome.

[kevinpapst]

I like the SVG solution in this case.

[cavasinf]

Ok, a little complication with this simple feature. Since the tooltip is a document level element, we cannot simply hide/show the a element with the title, because if the tooltip is already displayed, nothing will change.
Instead, we need to change the ariaLabel + data-bs-original-title attribute + the currently displayed tooltip.

[kevinpapst]

Hahaha, and that's why customer tend to say "but its only" ... without understanding all the problems that can arise from "only a link" 😁

Its is ready now from your end?

[cavasinf]

Hahaha, and that's why customer tend to say "but its only" ... without understanding all the problems that can arise from "only a link" 😁

Yeah, every time I open a PR, we always exchange with 15+ comments on it 😭
So I get the felling 😉

Its is ready now from your end?

• You can maybe add the de trans
• How do you generate the id prop for the trans? I saw you did it in your commit.

[kevinpapst]

The title attribute was still missing, I added the translations and the ids, which I generate with this command:
https://github.com/kimai/kimai/blob/main/src/Command/TranslationCommand.php

The command has many features which I need to manage translation files.

If you have no objections, I will merge and release

[cavasinf]

The title attribute was still missing

Yeah, I thought as long as the bootstrap tooltip is init, title is not really mandatory anymore since bootstrap will never read title again when initialized. aria-label was more important as it is used by screen readers.
But useful if one day bootstrap doesn't work anymore, the native browser tooltip will works.

The command has many features which I need to manage translation files.

Nice 👍

If you have no objections, I will merge and release

No objections 🫡

[kevinpapst]

I don't see the bootstrap title, as I use data-toggle instead of data-bs-toggle, which is obviously my own problem 😁 but that's why I immediately saw it.