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

Add show/hide button for password inputs #566 #664

Merged

Conversation

Qizot
Copy link
Contributor

@Qizot Qizot commented Oct 31, 2021

Closes #566

Added visibility toggle for the password input cell.

Screen.Recording.2021-10-31.at.13.01.41.mov

There is one problem with it though, as the project does not support any nice way of executing js code (I mean lightweight utilities like alpine.js to keep state of the button visibility) then when somebody shows the password and starts typing , SessionLive will overwrite the state of CellComponent that keeps information about input visibility. Let me know if this behaviour is acceptable or not (I think that this toggle should have local state instead of sessions' scoped, so to allow for update with visible password we would have to mix local state and session's state data which is ugly).

@jonatanklosko
Copy link
Member

jonatanklosko commented Nov 1, 2021

Thanks for working on this! So I think this shouldn't be a global state, and not necessarily a part of the socket either. Ideally we could re-use the toggle for other password inputs we may have (we already have some in the file systems configuration). It may be worth having a JS hook handling that, and we would drop that hook whenever necessary. We do need to render the toggle button, so one idea I have is to have a stateless component like this:

<.with_password_toggle>
  <input ...>
</.with_password_toggle>

The component would then render:

<div class="relative" phx-hook="PasswordToggle">
  <!-- the input -->
  <button class="absolute ..." phx-change="ignore">
    <!-- the icon element, we would change its class dynamically -->
  </button>
</div>

When the button gets clicked we would change the input type (text/password), and the only question is how to prevent LV from overriding the attribute on next render. I would try keeping info about the "toggled" state in the hook, and then in update() callback we always update type to reflect this state. This should work fine, but I also have another idea if it doesn't :)

Let me know if that makes sense.

@josevalim feel free to drop any other ideas :D

@Qizot
Copy link
Contributor Author

Qizot commented Nov 1, 2021

When the button gets clicked we would change the input type (text/password), and the only question is how to prevent LV from overriding the attribute on next render. I would try keeping info about the "toggled" state in the hook, and then in update() callback we always update type to reflect this state. This should work fine, but I also have another idea if it doesn't :)

This has potential to work, will try it and then will let you know :)

@Qizot
Copy link
Contributor Author

Qizot commented Nov 1, 2021

Seems to be working as expected now 😄

@jonatanklosko
Copy link
Member

Looks great, just a couple minor comments!

@Qizot
Copy link
Contributor Author

Qizot commented Nov 1, 2021

I think that in the end it came out together nicely 😸

@jonatanklosko
Copy link
Member

Beautiful! We have two password inputs in LivebookWeb.SettingsLive.AddFileSystemComponent, could you use the component there and verify that it also looks as expected?

@Qizot
Copy link
Contributor Author

Qizot commented Nov 1, 2021

Beautiful! We have two password inputs in LivebookWeb.SettingsLive.AddFileSystemComponent, could you use the component there and verify that it also looks as expected?

Done, works like a charm.

@jonatanklosko jonatanklosko merged commit ace64ea into livebook-dev:main Nov 1, 2021
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.

Add show/hide button for password inputs
2 participants