lukin / keywind

Keywind is a component-based Keycloak Login Theme built with Tailwind CSS
Apache License 2.0
779 stars 274 forks source link

feat: adding passwordVisibility button #56

Closed paulwer closed 9 months ago

paulwer commented 1 year ago

closes https://github.com/lukin/keywind/issues/13

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

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

paulwer commented 1 year ago

@lukin I just thought about puting the script tag, inside the input.ftl component to dont be forced to import it at each file, which had a password-input field. Any thoughts/suggestions about this?

i will check this and will come back here and/or say if it is ready to be merged.

paulwer commented 1 year ago

ready, can be merged <3

paulwer commented 11 months ago

@lukin any updates?

lukin commented 11 months ago

@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 commented 11 months ago

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 commented 11 months ago

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

paulwer commented 11 months ago

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 commented 11 months ago

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

paulwer commented 11 months ago

Possible, but I dont see, where I may missing somthing. The CSS styles are not dependent on other places of the branch and I copied the whole input component.

btw. the "error" is only present on screenwidth over sm. without the class sm:top-2 it works well at my side and I dont see a configuration, where the margins of the input component changes, so it have to be changed for the icon as well when screen size is large. Mobile works fine :)

but maybe i missed something :(

I am currently also quite busy, so I cannot support that much in testing. sorry. so if you like i would support merging it :)

paulwer commented 11 months ago

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

sorry for that :)

paulwer commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

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

paulwer commented 9 months ago

@lukin was the visual error fixed?

lukin commented 9 months ago

@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 commented 9 months ago

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

image

SimonScholz commented 9 months ago

@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.