Closed Bug 1550110 Opened 6 months ago Closed 4 months ago

Create CustomElement for a modal dialog and add 'Delete' warning dialog

Categories

(Firefox :: about:logins, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 69
Tracking Status
firefox69 --- fixed

People

(Reporter: jaws, Assigned: _6a68)

References

(Blocks 3 open bugs)

Details

(Whiteboard: [passwords:management] [skyline] )

Attachments

(1 file, 2 obsolete files)

The modal dialog will be used when there are general errors to be display the confirmation prompt when deleting a login and also when showing the Connect Another Device wizard.

Flags: qe-verify-
Type: task → enhancement
Assignee: nobody → jhirsch
Status: NEW → ASSIGNED

Create a generic modal dialog equipped with a show() method that allows
content and callbacks to be passed in dynamically.

Still TODO:

  • use the existing stylesheets from about:preferences instead of
    recreating the XUL dialog styles in the new component

  • write tests

  • figure out if anything needs to be done about localization, given that
    the content will be passed in by concrete modal implementations

  • many questions called out as TODOs in the code

  • Rewired the login-item delete flow to show the confirm-delete modal
    after clicking the Delete key. The signal is fired on 'OK' click.

  • The modal is canceled if the user hits Escape, the 'x' dismiss key,
    the 'Cancel' button, or clicks outside the dialog, on the darkened
    overlay. This matches the preferences subdialog input handling behavior.

  • Need to update delete test and add modal tests

  • Still need to convert CSS from fixed to flex

  • Not sure how to get fluent to insert all the l10n keys for an element
    created via JS; only the title attribute is currently getting inserted.

  • Not sure how to correctly insert slot content via JS; I'm currently
    appending to the slot, but I suppose I could replace the slot with the
    slot content. Not sure if the special slot-element properties would be
    set in that case, though :-\

  • After some back-and-forth, opted for inheritance over composition;
    composing the base dialog inside the specific dialog led to problems
    double-reflecting fluent strings. Inheritance also led to less code
    overall.

MozReview-Commit-ID: EVal0e0hJPa

Summary: Create CustomElement for a modal dialog → Create CustomElement for a modal dialog and add 'Delete' warning dialog
Attachment #9066585 - Attachment is obsolete: true
Attachment #9071468 - Attachment description: Bugs 1550110 and 1550103: Base modal and confirm-delete modal, WIP #2 → Bugs 1550110: Base modal and confirm-delete modal, WIP #2
  • Add tests (some issues simulating keystrokes, currently left as TODOs)

  • Start extracting duplicated test setup to a head.js file (some issues
    with cross-test pollution to finish debugging)

  • Accessibility fixes: set the dialog role and focus the cancel button
    when the dialog is shown.

  • Add confirm delete to delete login flow

  • Remove modal base class for now

MozReview-Commit-ID: 9TlO86gLhPV

Attachment #9071468 - Attachment is obsolete: true
Attachment #9074014 - Attachment description: Bug 1550110 - Confirm Delete Dialog, now without a separate base class → Bug 1550110 - CustomElement for a modal dialog and add 'Delete' warning dialog
Pushed by jhirsch@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6fa0472457f
CustomElement for a modal dialog and add 'Delete' warning dialog r=jaws,fluent-reviewers,Pike
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8fa127ef3d04
CustomElement for a modal dialog and add 'Delete' warning dialog r=fluent-reviewers,Pike
Flags: needinfo?(jhirsch)
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Whiteboard: [passwords:management] [skyline]
Component: Password Manager → about:logins
Product: Toolkit → Firefox
Target Milestone: mozilla69 → Firefox 69
You need to log in before you can comment on or make changes to this bug.