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

[FedCM] Show account chooser for non-returning user on approved client #46707

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jun 12, 2024

Currently on button mode, for this particular case of a logged out
user signing in as a non-returning user on an RP which is in the list
of approved clients, we would show disclosure UI without the
disclosure text. This patch makes it such that we show the account
chooser instead because the disclosure UI without the disclosure text
is unnecessary new UI surface which doesn't convey any additional
information compared to the account chooser.

Note that we show the multi account picker instead of the single account
picker. This is because when we were showing disclosure UI, the user was
able to go back and retrieve the original list of accounts by clicking
the "Back" button. However, the "back" button is not available on the
account pickers. We prefer a multi account picker compared to having an
additional UI surface. The most recently signed in accounts are shown
at the top of the multi account picker.

This patch also refactors the state machine. We used to set the state_
to State::REQUEST_PERMISSION for single account pickers on the bubble
but that is confusing. This patch changes it to have state_ be
State::SINGLE_ACCOUNT_PICKER.

Bug: 342557704
Change-Id: I9435ee5a34cf47567c6a4a16bbffd99ba8eee798
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5578254
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Zachary Tan <tanzachary@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313829}

Currently on button mode, for this particular case of a logged out
user signing in as a non-returning user on an RP which is in the list
of approved clients, we would show disclosure UI without the
disclosure text. This patch makes it such that we show the account
chooser instead because the disclosure UI without the disclosure text
is unnecessary new UI surface which doesn't convey any additional
information compared to the account chooser.

Note that we show the multi account picker instead of the single account
picker. This is because when we were showing disclosure UI, the user was
able to go back and retrieve the original list of accounts by clicking
the "Back" button. However, the "back" button is not available on the
account pickers. We prefer a multi account picker compared to having an
additional UI surface. The most recently signed in accounts are shown
at the top of the multi account picker.

This patch also refactors the state machine. We used to set the state_
to State::REQUEST_PERMISSION for single account pickers on the bubble
but that is confusing. This patch changes it to have state_ be
State::SINGLE_ACCOUNT_PICKER.

Bug: 342557704
Change-Id: I9435ee5a34cf47567c6a4a16bbffd99ba8eee798
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5578254
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Zachary Tan <tanzachary@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313829}
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 6850027 into master Jun 12, 2024
21 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-5578254 branch June 12, 2024 05:32
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.

None yet

3 participants