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

fix: can focus combobox items (#679) #689

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

yeonsubak
Copy link
Contributor

@yeonsubak yeonsubak commented Nov 16, 2024

/claim #679

description

  • Update @radix-ui/react-popover to ^1.1.2 that eventually fix the focusing issue of the combobox items.

related issue: #679 #654

type of change

  • bug fix
  • new feature
  • breaking change
  • documentation update

how to test

  1. Make sure to have "@radix-ui/react-popover": "^1.1.2" under dependencies in /screenpipe-app-tauri/package.json
  2. Run bun install from /screenpipe-app-tauri in your terminal to update @radix-ui/react-popover
  3. Run bun tauri dev
  4. Once the application has started, go to Settings menu and check the following combobox elements are properly working
    • monitors
    • audio devices
    • languages

if relevant add screenshots or screen captures to prove that this PR works to save us time.

screenpipe-pr-issue-679.mp4

checklist

  • MOST IMPORTANT: this PR will require less than 30 min to review, merge, and release to production and not crash in the hand of thousands of users
  • i have read the CONTRIBUTING.md file
  • i have updated the documentation if necessary
  • my changes generate no new warnings
  • i have added tests that prove my fix is effective or that my feature works

additional notes

I tried inspecting related components from shadcn library but they all looked good and don't seem to cause the issue. Just out of the intuition, I suspected that it's caused by the Popover component of radix-ui so I updated it to the latest version and it eventually worked.

- Update @radix-ui/react-popover to ^1.1.2 that eventually fix the focusing issue of the combobox items.
Copy link

vercel bot commented Nov 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 16, 2024 2:57pm

@louis030195
Copy link
Collaborator

/approve

amazing thanks

Copy link

algora-pbc bot commented Nov 17, 2024

Could not find a claim that corresponds to the PR. Please add the /claim snippet to the PR body first.

@louis030195 louis030195 merged commit c26cbf3 into mediar-ai:main Nov 17, 2024
2 checks passed
@louis030195
Copy link
Collaborator

@yeonsubak can you add the claim for bounty plz

@algora-pbc algora-pbc bot mentioned this pull request Nov 17, 2024
Copy link

algora-pbc bot commented Nov 17, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe.

@yeonsubak
Copy link
Contributor Author

@yeonsubak can you add the claim for bounty plz

Thanks! I added the claim on my PR body.

@yeonsubak yeonsubak deleted the fix-focus-issue-#679 branch November 17, 2024 20:23
@louis030195
Copy link
Collaborator

/approve

Copy link

algora-pbc bot commented Nov 17, 2024

@louis030195: The claim has been successfully added to reward-all. You can visit your dashboard to complete the payment.

@louis030195
Copy link
Collaborator

Screen.Recording.2024-11-18.at.11.00.29.AM.mov

i did not test the change and actually it did not work ...

@yeonsubak
Copy link
Contributor Author

yeonsubak commented Nov 18, 2024

Screen.Recording.2024-11-18.at.11.00.29.AM.mov
i did not test the change and actually it did not work ...

Thanks for the feedback! I will check it out.
Can you explain how to navigate to that page in the video or list other pages that I can look into?

First, let me try modify Command component as I introduced in shadcn's PR. If it doesn't work, I will thoroughly inspect the elements and their dynamics in the page.

@yeonsubak
Copy link
Contributor Author

yeonsubak commented Nov 18, 2024

@louis030195 Actually, I ran both bun tauri dev and compiled screenpipe-app.exe without modifying any codes from commit 478fb05 and it seems to be working well.

Could you please check whether the dependency update actually happened in your CI/CD?

how to test

  1. Make sure to have "@radix-ui/react-popover": "^1.1.2" under dependencies in /screenpipe-app-tauri/package.json
  2. Run bun install from /screenpipe-app-tauri in your terminal to update @radix-ui/react-popover
screenpipe-issue-679-2.mp4
screenpipe-issue-679-3.mp4

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.

2 participants