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

Desktop: Resolves #9553: Added enter key support to navigate search results #10013

Closed
wants to merge 4 commits into from

Conversation

njmulsqb
Copy link
Contributor

@njmulsqb njmulsqb commented Feb 27, 2024

Fixes #9553

@PackElend label me please, apologies pre-commit hooks didnt execute on previous PR, it was some issue with my Windows machine, now creating this PR with macbook and pre-commits hook executed successfully

Introduction URL: https://discourse.joplinapp.org/t/introducing-najam/36216

With 💚 from Tecvity

njmulsqb and others added 4 commits February 27, 2024 13:58
Signed-off-by: Najam Ul Saqib <najam.saqib@constellationdealer.com>
Signed-off-by: Najam Ul Saqib <najam.saqib@constellationdealer.com>
Copy link
Contributor

github-actions bot commented Feb 27, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@laurent22
Copy link
Owner

it was some issue with my Windows machine

We have heard of this issue before but never been able to replicate it or fix it. Would you mind trying to commit again on your Windows machine and see what happens - is there any error or warning? Could you provide a screenshot if there's something?

@laurent22
Copy link
Owner

Also all the commits need to be under the same username for the CLA to be valid. So you might want to squash all the commits under the CLA name

@njmulsqb
Copy link
Contributor Author

recheck

@njmulsqb
Copy link
Contributor Author

is there any error or warning? Could you provide a screenshot if there's something?

The pre-commit hook is failing on windows
image
I had to use --no-verify bypass in previous PR

@laurent22
Copy link
Owner

Thanks for checking. I'm not sure why yarn updateIgnored would fail here. Would you mind checking this?

  • What version of yarn are you using? yarn --version should tell you
  • And what if you run yarn run updateIgnored?

@njmulsqb
Copy link
Contributor Author

* What version of yarn are you using? `yarn --version` should tell you

3.6.4

* And what if you run `yarn run updateIgnored`?

image

@laurent22
Copy link
Owner

Oh I see it now, that script should be executed from the root of the repository, so in your case from C:\GitHub\joplin. I will improve that error message to make that clearer

@laurent22
Copy link
Owner

Now the problem is that I'm not sure what should be implemented here. Feel free to participate in the discussion at #9553 to figure this out

@laurent22
Copy link
Owner

Closing for now. We need to define what we want to implement (if anything)

@laurent22 laurent22 closed this Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arrow keys cannot be used to navigate search results
3 participants