Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

Add tabIndex attribute to scroll to top btn #992

Merged
merged 2 commits into from
Jan 23, 2022

Conversation

BSlaven
Copy link
Member

@BSlaven BSlaven commented Jan 14, 2022

Closes #990

Simply adding tabIndex attribute to the button did not work in the sense that pressing Enter did not trigger the function.

My solution was to add onKeyPress attribute to the button and pass it the same function as for onClick.

This of course meant that I had to make sure that only pressing 'Enter' and clicking on the button trigger that function.
Hence the condition.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

It's great having you contribute to this project

Welcome to the community 🤓

If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.

Copy link
Member

@EmmaDawsonDev EmmaDawsonDev left a comment

Choose a reason for hiding this comment

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

Ah yes, that makes sense! Thanks for spotting the need for a key press and adding it in. LGTM!

@digambar-t7
Copy link
Contributor

Hey, can you remove line no 29 and check if it is significant

@BSlaven
Copy link
Member Author

BSlaven commented Jan 15, 2022

Actually line 29 is rather useless. With or without it functionality seems to work fine both on homepage and on profile page.
Also both by pressing enter or clicking on it.
Should I remove it? Thoughts?

@digambar-t7
Copy link
Contributor

Being the creator of the scroll-to-top-btn, I think that line no.29 should be erased because I don't remember the grounds on which I included it.
But, let's have a confirmation from others.

@EmmaDawsonDev
Copy link
Member

Yes, go ahead and remove it if it’s not serving any purpose

@BSlaven
Copy link
Member Author

BSlaven commented Jan 16, 2022

I removed the unnecessary line.

@EmmaDawsonDev EmmaDawsonDev merged commit 0666fab into EddieHubCommunity:main Jan 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

To Top button not accessible with keyboard
5 participants