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

Add support for detecting function keys #2

Open
pranesh239 opened this issue Oct 6, 2020 · 15 comments
Open

Add support for detecting function keys #2

pranesh239 opened this issue Oct 6, 2020 · 15 comments
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest

Comments

@pranesh239
Copy link
Owner

as of now use-key-capture doesn't detect functional keys (F1, F2 and so on). If you wish, you can add this feature.

@pranesh239 pranesh239 added enhancement New feature or request good first issue Good for newcomers hacktoberfest labels Oct 6, 2020
@muhameddelic
Copy link

Hi, I can take a look at this :)

@saurabh-172
Copy link

hi @pranesh239 I'm new to opensource contribution and hacktoberfest I would like to work on this can you help me with how to start with this.
This would be my first contribution and I don't want it to become spam and get disqualified (little afraid)

@pranesh239
Copy link
Owner Author

@muhameddelic sure, give this a try

@pranesh239
Copy link
Owner Author

@saurabh-172 cool, nothing to worry.

fork this repo -> git clone forked repo to your local -> look at the code -> make the necessary change -> push to your branch -> from your branch raise a PR.

Give this a try, if you stuck somewhere please comment here. I can help you.

@saurabh-172
Copy link

saurabh-172 commented Oct 8, 2020

@pranesh239
I tried to understand the code and I think I need to code the part where the reset functionality is required but I had a doubt that resetting will be executed using escape key so when the escape key is pressed in global listener format I need to set type as "ESCAPE_KEY" and when in target listener format we need to set type as "RESET_CAPTURES" but then how do get to now that getTargetProps is not created i.e. global listener is created because no matter how we create the listener the getTargetProps will always returned and it only depends on the creater that he added the 2nd argument i.e. getTargetProps to get it??

@saurabh-172
Copy link

@pranesh239
According to what I understood I have made some changes in code but if I do pull request then it will be like my finale submission right so isn't there a way that you can confirm that the changes I have made are correct or not?

@muhameddelic
Copy link

@saurabh-172 no, a PR is not a final submission. The point of a PR is that you're requesting that you have the changes you've made merged to a certain target branch. This is a good time for maintainers & other collaborators to review the code and possibly leave some feedback. It's not about critiquing someone's code so there's no need to feel like you have to make the perfect PR.

@saurabh-172
Copy link

saurabh-172 commented Oct 9, 2020

@pranesh239 I made some changes and have made a pull request so If anything is wrong please help me understand it, as I'm new to react hooks but I read about them and practiced too before working on it. So that accordingly I will change it, again and again, do a Pull request after correction.

@muhameddelic thanks for clearing my doubt

@pranesh239
Copy link
Owner Author

@saurabh-172 that's amazing what you did, but on ESCAPE_KEY we can't reset the data. In that case, if user wants to listen for ESCAPE_KEY then it won't work.

And as you are new to open source I suggest you a way of raising a PR.

  1. Open an issue on what you found as an issue.
  2. If you have a solution please mention in the issue.
  3. If the author or maintainer agree on what you raised as an issue and your solution, then you will be assigned to do the need.
  4. Then you raise your solution as PR.

Don't spend time on raising PR without the knowledge of author.

@saurabh-172
Copy link

So now should I remove the PR which I made and then separately raise the issue Is this possible. Or you will just check and reply that the changes I are correct or wrong and if they are wrong then again instead of sending the PR I should 1st raise an issue then keep uploading codes in that issue and once the solution is approved by you then make PR.

which option do I have now from above that I mentioned as I made a PR now? Is it possible to remove that PR and make a issue of that instead?

And when I was searching for some issues to solve related to hactoberfest I came across some issues which were already assigned so if someone is assigned to that issue then are we allowed to ask like we want to contribute to that and make PR or raise issue ?? I mean can there be more than one assigned to one issue or task in hactoberfest. or I just need to search for those who are not assigned?

@muhameddelic
Copy link

@pranesh239 @saurabh-172 My apologies, I didn't even realise it wasn't assigned to anyone, but yes it should depend on the author/maintainers' preference.

I still don't think it's end of the world that the PR was raised, it can always be closed & re-opened after an agreement has been reached. That's what these tools are for 😄

@pranesh239 pranesh239 assigned pranesh239 and unassigned pranesh239 Oct 9, 2020
@saurabh-172
Copy link

@pranesh239 I checked once again and I understood that the 2nd condition in getAction() with keyCodeMapper was already listening to ESCAPE_KEY no matter which type of listener was made so sorry for the wrong understanding and also the reset key type is already been assigned but the payload for it is not being assigned so I think that's what is to be added. Is this what I understood is correct if it is then as you mentioned I will open an issue and tell in detail about it and try to solve it if I could.

Also as you mentioned and again sorry for PR before confirming, I removed the PR which I made but no need to delete the branch right I asked because it was showing that option?

But still, I didn't understood why that extra useEffect hook is being used for checking is_TRUSTED if it is allowed to ask this so please explain to me?

@saurabh-172
Copy link

saurabh-172 commented Oct 11, 2020

@pranesh239
as you mentioned I have raised an issue related to what I think is missing and what I would be able to solve and also asked some doubts. I studied accordingly and have raised the issue if any mistakes please tell. please do check the issue.

@pranesh239
Copy link
Owner Author

@saurabh-172 great! I will check the PR and come back to you. Great work!!!

@saurabh-172
Copy link

saurabh-172 commented Oct 12, 2020

@pranesh239 wait that one is the previous PR I closed it I don't know how you can see when you mentioned about opening an issue I opened an issue but not made any PR I was waiting for your response on that issue which I created that my issue raised is valid or not. So this PR I made previously. please check my issue which I raised and if you say that's correct then I will make a PR related to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

No branches or pull requests

3 participants