-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[BEEEP][PM-14388] Better dev experience on desktop-browser IPC #11822
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #11822 +/- ##
==========================================
- Coverage 33.55% 33.53% -0.02%
==========================================
Files 2814 2814
Lines 87435 87464 +29
Branches 16674 16677 +3
==========================================
- Hits 29337 29330 -7
- Misses 55793 55829 +36
Partials 2305 2305 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, one question, is there any world where a dev would want to opt out of this improvement?
For example in browser we don't show the welcome tab on install but we added a way to opt out of that so you can test the more production like experience.
I think this differs in that it's actually making the dev experience MORE like production but just wanted to offer it up as a question.
|
||
// If one of the commands is autofill_login or generate_password, we know it's probably the Bitwarden extension | ||
for (const { command_name, extension } of Object.values(commands)) { | ||
if (command_name === "autofill_login" || command_name === "generate_password") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Genius
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@quexten should get the props for this idea, he's the one who came up with it 😄
I think most of the changes as you mention only bring us closer to what production should be, and the differences that these changes enable aren't really behavioral, they just unblock certain features that previously weren't working in dev, so I can't imagine there's many reasons to disable them. For testing a more production build on MacOS, you'll need to test against a |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-14388
📔 Objective
This PR tries to improve developer experience on desktop-browser integration by removing some unnecessary steps.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes