[ALPC Extensions]: Add new "Request Parent Approval" dialog [chromium/src : main]
Richard Chui (Gerrit)
Richard Chui added 1 comment![]( "Open in Gerrit")
.SetStyle(ui::ButtonStyle::kProminent))@emiliapaz: based on what I see in the extensions designs, eventually the styling will update to do something similar to this. I haven't seen it based on my ToT build yet, so just wanted to make sure that this is the case, so this new dialog won't be inconsistent.
Related details
- Emilia Paz
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emilia Paz (Gerrit)
Emilia Paz added 8 comments![]( "Open in Gerrit")
Thanks Richard! We may also want a test here, although simpler one
} else if (supervised_user::AreExtensionsPermissionsEnabled(profile_) &&
!supervised_user::SupervisedUserCanSkipExtensionParentApprovals(
profile_)) {Can we explain in the CL description why do we want this?
base::BindOnce(&WebstorePrivateBeginInstallWithManifest3Function::
RequestExtensionApproval,
this, web_contents));Web contents could be null when this gets called, is this handled? Maybe we need a separate `OnRequestParentApprovalPromptAccepted`. Or maybe same callback for accept/cancel that recieves input from the dialog selection
base::BindOnce(&WebstorePrivateBeginInstallWithManifest3Function::
OnRequestParentApprovalPromptDone,
this),
base::BindOnce(&WebstorePrivateBeginInstallWithManifest3Function::
RequestExtensionApproval,
this, web_contents));`ShowExtensionInstallRequestParentApprovalDialog` has first the cancel callback, and then the ok callback. However here, the first one is OnDone, and the second one Approval. Is this intended? If so, it's a bit confusing
OnRequestParentApprovalPromptDone() {This is for the cancel callback, lets rename it so its more clear
.SetStyle(ui::ButtonStyle::kProminent))@emiliapaz: based on what I see in the extensions designs, eventually the styling will update to do something similar to this. I haven't seen it based on my ToT build yet, so just wanted to make sure that this is the case, so this new dialog won't be inconsistent.
Does adding the OkButton without style doesn't default to be the "main" one? My understanding is that DialogModel should have default styles for the buttons and we shouldn't modify them unless there is a reason why UX is requesting that
// Shows a dialog to notify the user that they need to ask their parent for
// approval to install an extension.
void ShowExtensionInstallRequestParentApprovalDialog(
content::WebContents* web_contents,
base::OnceClosure cancel_callback,
base::OnceClosure ok_callback);BUILDFLAG(IS_ANDROID)
Related details
- Richard Chui
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |