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

feat: adding passwordVisibility button #56

Merged
merged 5 commits into from
Dec 13, 2023
Merged

Conversation

paulwer
Copy link
Contributor

@paulwer paulwer commented Sep 25, 2023

closes #13

this pr is related to the keycloak visibility behavior used in the base theme.

270369066-f4070fbd-47f5-4578-9ebf-7d828e7b08b2

@paulwer

This comment was marked as outdated.

@paulwer
Copy link
Contributor Author

paulwer commented Sep 26, 2023

ready, can be merged <3

@paulwer
Copy link
Contributor Author

paulwer commented Oct 24, 2023

@lukin any updates?

@lukin
Copy link
Owner

lukin commented Oct 24, 2023

@paulwer Sorry for the wait. I was currently working on a Changelog based on Conventional Commits. 🙂

For consistency, we need to replace the icons with heroicons.
I think if we use Alpine.js, we won't need a separate script.

@lukin
Copy link
Owner

lukin commented Oct 24, 2023

I'm planning to do some refactoring of this PR this week. Unfortunately, I can't devote much time to the project for now, so I'm prioritising trying to automate all the required and long-awaited processes. 🙂

@lukin
Copy link
Owner

lukin commented Oct 29, 2023

@paulwer Please, if possible, check that everything works correctly on your side, and I'll merge it.

@paulwer
Copy link
Contributor Author

paulwer commented Oct 30, 2023

there is too small margin on top of the icon, so its not displayed in the middle of the field. So this have to be fixed.
image

But its a even better implementation. I like it <3
after this, it could be merged from my pov. :)

@lukin
Copy link
Owner

lukin commented Oct 30, 2023

I suspect it has to do with your custom styles. Please make sure you test the branch without modifications. 🙂

@paulwer

This comment was marked as outdated.

@paulwer
Copy link
Contributor Author

paulwer commented Oct 31, 2023

@lukin sometimes "sleeping" over it helped. The component works like a charm. It was a human error instead ❤️

sorry for that :)

@paulwer
Copy link
Contributor Author

paulwer commented Nov 4, 2023

Please verify this works on mobile. (Google Pixel 5)
Currently I am on vacation, but found the following:
Screenshot_20231104_223615_Chrome.jpg

Not sure, if this is the case in your PR, but lets doublecheck at least :)

@anshuman852
Copy link

anshuman852 commented Nov 15, 2023

Hi, if i were to pick your commit for this, how do i find it? all of your pull requests show like 76 commits
Edit- nvm found the commits, its quite weird on ur commit history the old commits show on top and the new ones on bottom
I cherrypicked here https://github.com/anshuman852/keywind/commits/master , for some reason it doesnt show ur profile on "authored"

@paulwer
Copy link
Contributor Author

paulwer commented Nov 28, 2023

@lukin should I try to fix the optical issue or is this your task now?

@paulwer
Copy link
Contributor Author

paulwer commented Dec 13, 2023

@lukin was the visual error fixed?

@lukin
Copy link
Owner

lukin commented Dec 13, 2023

@paulwer I have not been able to reproduce your error, and it is not reproducible in HTML templates. I suspect it has to do with cache or missing styles.

@SimonScholz
Copy link

After updating to the latest version I now have this "visual error" as well:

image

@SimonScholz
Copy link

@paulwer I have not been able to reproduce your error, and it is not reproducible in HTML templates. I suspect it has to do with cache or missing styles.

Sorry for the confusion, for me it was indeed a caching issue. Cloudflare has cached the css and therefore it did not work.

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.

Show-hide Password button
4 participants