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

bug fix: "Active" button not getting focused state #384

Merged

Conversation

afzalzbr
Copy link
Contributor

@afzalzbr afzalzbr commented Apr 9, 2023

The focusOutline was not including any case for active === true.

I've made changes in the file: src\Button\Button.tsx:
image

Following is the working example of the fix for the 'Active' button, followed by a picture showing no other buttons' styles are being compromised:
image

image

I hope this helps!

Fixes #383

@arturbien
Copy link
Member

@afzalzbr there are some linting issues in CI

@afzalzbr
Copy link
Contributor Author

@arturbien okay I'll check.

afzalzbr added a commit to afzalzbr/React95 that referenced this pull request Apr 13, 2023
@afzalzbr
Copy link
Contributor Author

@arturbien fixed the linting issue, please review and approve.

Copy link
Member

@arturbien arturbien left a comment

Choose a reason for hiding this comment

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

Seems like we can make this PR a lot simpler.

Just remove the !active flag on line 167 instead of adding the code you've added. I think the result will be the same right?

https://github.com/react95-io/React95/pull/384/files#diff-27a5747646c8932c6f4df16f6a7882c2f7a9481111c57cb83944df1c58b3c7bdR167

@afzalzbr
Copy link
Contributor Author

@arturbien yup, you are right. I removed it and it's working fine now.

arturbien
arturbien previously approved these changes Apr 21, 2023
@arturbien
Copy link
Member

@afzalzbr great! Now before I merge please squash all the commits into single commit with message:

fix(Button): active button focus outline

bug fix: active button focus issue resolved

removed !active flag from line 167.

bug fix: "Active" button not getting focused state
@afzalzbr
Copy link
Contributor Author

@arturbien done

@arturbien arturbien merged commit 871d533 into react95-io:master Apr 24, 2023
@arturbien
Copy link
Member

@afzalzbr your commit was merged to master. Thank you for the contribution <3

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.

"Active" button not getting focused state (dashed inner border)
2 participants