-
Notifications
You must be signed in to change notification settings - Fork 91
fix(onboarding): Delay the keycard detection for when keycard is needed #19547
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
base: fix/keycard-nfc-1
Are you sure you want to change the base?
Conversation
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19547#1 🔹 ~15 min 🔹 39ed060 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prs-windowsPR19547 🔹 ~22 min 🔹 39ed060 🔹 📦 tests/e2e-windows package |
3eacf69 to
1bb9a1e
Compare
39ed060 to
d281472
Compare
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19547#2 🔹 ~24 min 🔹 d281472 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19547 🔹 ~23 min 🔹 d281472 🔹 📦 tests/e2e package |
1bb9a1e to
e9d79a4
Compare
d281472 to
e64335b
Compare
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19547#3 🔹 ~1 hr 6 min 🔹 e64335b 🔹 📦 tests/ui package |
e64335b to
92e99e9
Compare
✔️ status-app/prs/android/arm64/package/PR-19547#4 🔹 ~11 min 🔹 1e73bf1c 🔹 📦 android/arm64 package |
✔️ status-app/prs/linux/x86_64/tests-nim/PR-19547#4 🔹 ~18 min 🔹 92e99e9 🔹 📦 tests/nim package |
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19547#4 🔹 ~23 min 🔹 92e99e9 🔹 📦 tests/ui package |
e9d79a4 to
58d65b8
Compare
92e99e9 to
397f697
Compare
✔️ status-app/prs/linux/x86_64/tests-nim/PR-19547#5 🔹 ~17 min 🔹 397f697 🔹 📦 tests/nim package |
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19547#5 🔹 ~22 min 🔹 397f697 🔹 📦 tests/ui package |
caybro
left a comment
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.
Cool :) Would be nice to add a small QML test to tst_OnboardingLayout.qml
58d65b8 to
f2eb2e9
Compare
397f697 to
41159d3
Compare
Jenkins BuildsClick to see older builds (50)
|
| error "error storing metadata", err=e.msg | ||
|
|
||
| proc startDetection*(self: Service) {.featureGuard(KEYCARD_ENABLED).} = | ||
| self.asyncStart(status_const.KEYCARDPAIRINGDATAFILE) |
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.
Should we ignore the signal if there is a previous detection in progress?
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.
Not necessarily. Start can be called at any point even if the API was previously started. So the keycard client doesn't need to track the API state. The result will be a re-read of keycard to get the current metadata. It's probably something we'll want either way.
friofry
left a comment
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 good to me!
Added a comment
f2eb2e9 to
a18039a
Compare
This is a fix for the mobile platforms that will show a drawer when the keycard is needed. We'll need to avoid showing the drawer every time at app start.
41159d3 to
d606f25
Compare
What does the PR do
Iterates #19545 #19546
This is a fix for the mobile platforms that will show a drawer when the keycard is needed. We'll need to avoid showing the drawer every time at app start.
The UI will signal the onboarding module whenever the keycard is needed to start the session API.
NOTE: Tests can be done in the final PR #19549