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.

Sign up for GitHub

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

Jump to bottom

Fix inputs with show/hide password icon #3045

Merged
merged 19 commits into from
Jul 7, 2024
Merged

Fix inputs with show/hide password icon #3045

merged 19 commits into from
Jul 7, 2024

Conversation

samaradel
Copy link
Contributor

@samaradel samaradel commented Jun 26, 2024 •
edited
Loading

Description

  • Fix show/hide icon selection when pressing tab
  • Fix the order of dialog actions tabindex

Changes

Screencast.from.26-06-24.17.10.30.webm

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

samaradel added 2 commits June 26, 2024 17:00
@samaradel
- Remove password input wrapper
d957cee 
- Create prepend inner icon for inputs with type password
- Fix show/hide icon selection when press tab
- Fix the order of dialog actions tabindex
@samaradel
Remove password input wrapper exporting from hooks
befc351
@MohamedElmdary
Copy link
Member

@samaradel

PasswordInputWrapper component was created to avoid multiple define of showX variables. however this is not only the case but also to avoid defined v-slot:append-inner over and over.
Also it's not only used in profile manager but also in multiple solution/apps for example k8s and it's currently broken.
image

I would suggest extend usePasswordInput hook to get a ref from password input itself then use that ref in onMounted to append tabindex="-1" attribute in icon which should fix the issue in the entire app without any breaks.

Also Currently if we click on input then tab it will popup to the manual > Login > input again > the manual > So on... won't focus any button which definitely not fixing this issue at all

@MohamedElmdary
Copy link
Member

image

amiraabouhadid
amiraabouhadid reviewed Jun 27, 2024
View reviewed changes
Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please merge development_2.6 to resolve this issue
image

@samaradel samaradel marked this pull request as draft June 27, 2024 08:41
@samaradel samaradel marked this pull request as ready for review July 1, 2024 11:45
@samaradel samaradel requested a review from amiraabouhadid July 1, 2024 11:45
@MohamedElmdary
Copy link
Member

@samaradel This solution is not generic to all password inputs so the issue still presented in a lot of inputs in application

@samaradel samaradel marked this pull request as draft July 2, 2024 10:20
Base automatically changed from development_2.6 to development July 2, 2024 12:42
@samaradel samaradel marked this pull request as ready for review July 4, 2024 11:54
Mahmoud-Emad
Mahmoud-Emad approved these changes Jul 4, 2024
View reviewed changes
Copy link
Contributor

@Mahmoud-Emad Mahmoud-Emad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@samaradel samaradel merged commit 67d1fc1 into development Jul 7, 2024
4 checks passed
@AhmedHanafy725 AhmedHanafy725 deleted the development_2.6_password_icon branch July 9, 2024 08:41
@xmonader xmonader added this to the 2.6.0 milestone Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MohamedElmdary MohamedElmdary MohamedElmdary approved these changes

@Mahmoud-Emad Mahmoud-Emad Mahmoud-Emad approved these changes

@AhmedHanafy725 AhmedHanafy725 Awaiting requested review from AhmedHanafy725 AhmedHanafy725 is a code owner

@zaelgohary zaelgohary Awaiting requested review from zaelgohary zaelgohary is a code owner

@maayarosama maayarosama Awaiting requested review from maayarosama maayarosama is a code owner

@AlaaElattar AlaaElattar Awaiting requested review from AlaaElattar AlaaElattar is a code owner

@0oM4R 0oM4R Awaiting requested review from 0oM4R 0oM4R is a code owner

@mohamedamer453 mohamedamer453 Awaiting requested review from mohamedamer453

@amiraabouhadid amiraabouhadid Awaiting requested review from amiraabouhadid amiraabouhadid is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.
5 participants
@samaradel @MohamedElmdary @amiraabouhadid @Mahmoud-Emad @xmonader