fix(Onboarding/HomePage): hide keycard related stuff on mobile #18933
Conversation
caybro
requested review from
a team,
alexjba,
micieslak and
noeliaSD
as code owners
Jenkins BuildsClick to see older builds (59)
|
| image: currentEntry.item.thumbnailImage | ||
| colorId: currentEntry.item.colorId | ||
| keycardCreatedAccount: currentEntry.item.keycardCreatedAccount | ||
| keycardCreatedAccount: currentEntry.item.keycardCreatedAccount && root.isKeycardEnabled |
There was a problem hiding this comment.
I don't think this is correct. If an account was created using Keycard and Keycard is not enabled, we actually need to show a warning or something.
Basically, the account will not be usable, because the keys are inside the Keycard.
So maybe instead we should disable the entry and add a tooltip (that shows on click because of mobile)
There was a problem hiding this comment.
Alright, I see, so it needs to be filtered out; will fix
There was a problem hiding this comment.
But on a second thought, how could this happen? We never supported creating Keycard accounts on mobile before, so this is not a problem at all? Such a keycard entry would never appear then in the profile selector
There was a problem hiding this comment.
But on a second thought, how could this happen? We never supported creating Keycard accounts on mobile before, so this is not a problem at all? Such a keycard entry would never appear then in the profile selector
There is one way it can happen and it's if someone pairs a keycard account from Desktop to Mobile. Then that account is gonna be unusable until we implement keycard support.
We'll add warnings on the syncing page to warn against doing it, but I don't think there is another easy way to stop it from happening
There was a problem hiding this comment.
I see... so either filtering such accounts off (removing them from the selector, which I just did locally), or just making it disabled (but the LoginSelector was never designed or developed with the possibility of having some disabled items in it...)
There was a problem hiding this comment.
Or something like this:
Zaznam.obrazovky.z.2025-10-01.16-49-51.mp4
There was a problem hiding this comment.
Fixed by filtering out the Keycard profiles if we have any and displaying this little warning icon with a tooltip:
caybro
force-pushed
the
18920-mobile-hide-keycard-user-flows
branch
2 times, most recently
from
cf7ca85 to
be47a8d
Compare
There was a problem hiding this comment.
I agree with @jrainville that it's wrong to hide the accounts completely.
It would look for the user like an account has disappeared. It would be more friendly to make them disabled, or indeed show some text next to it. Perhaps similar what you did with a red keycard icon, but I'd do it more explicit, like a red label instead of the password input, and the log in button disabled.
Though... I'm not sure how would a user even get to such case 🤔 If the keycard is not supported, how can the user get a keycard account installed?
I'm approving the PR anyway, as I'm not a stake holder here and there's Jo request changes already.
| model: root.loginAccountsModel | ||
| currentKeycardLocked: root.keycardState === Onboarding.KeycardState.BlockedPIN || | ||
| root.keycardState === Onboarding.KeycardState.BlockedPUK | ||
| isKeycardEnabled: root.isKeycardEnabled |
There was a problem hiding this comment.
I'd rather call it keycardSupported 🤔
There was a problem hiding this comment.
Well, it matches the FeatureFlagsStore.keycardEnabled; it's not really a question whether it's supported, but rather if enabled (by the respective feature flag)
caybro
force-pushed
the
18920-mobile-hide-keycard-user-flows
branch
from
be47a8d to
d4e6599
Compare
@igor-sirotin I explained how it can happen here: #18933 (comment) |
caybro
force-pushed
the
18920-mobile-hide-keycard-user-flows
branch
2 times, most recently
from
b572d9e to
1665659
Compare
Done @jrainville & @igor-sirotin, we show them disabled with a red keycard icon + tooltip: 
Such items cannot be selected |
Those entries will be disabled (mostly) on platforms with no mouse pointer. So the info is inaccessible. What about putting that directly into the delegate (with some design team help probably)? |
caybro
force-pushed
the
18920-mobile-hide-keycard-user-flows
branch
from
1665659 to
fa39c71
Compare
OK going for a second line of (smaller) text: |
- drop reference to macOS/Intel -> no longer supported
- based on the respective feature flag - hide the controls instead of visually disabling them
- based on the respective feature flag - hide the controls instead of visually disabling them, except for the login profile selector - adjust the tests and SB pages - fix the "paste" helpers in the Onboarding SB page Fixes #18920
caybro
force-pushed
the
18920-mobile-hide-keycard-user-flows
branch
from
fa39c71 to
d2be288
Compare
6 participants
What does the PR do
(plus some minor fixes in separate commits)
Fixes #18920
Affected areas
Onboarding,Homepage
Architecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
Keycard enabled on Login:
Keycard disabled on Login:
Keycard enabled in Onboarding:
Keycard disabled in Onboarding:
Homepage:
Homepage disabled:
Impact on end user
The keycard related stuff is hidden from the onboarding/login/homepage views