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

Status: RESOLVED FIXED

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

Description

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 1

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: WIP modal dialog component implementation

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

Comment 2

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Bugs 1550110: Base modal and confirm-delete modal, WIP #2

• 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

Comment 3

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Bug 1550110 - CustomElement for a modal dialog and add 'Delete' warning dialog

• 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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6fa0472457f
CustomElement for a modal dialog and add 'Delete' warning dialog r=jaws,fluent-reviewers,Pike

Comment 5

[Razvan Maries]

Backed out for ES Lint failure.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=a6fa0472457f8406f3cac75c777cfeb5770913e2&selectedJob=254489960

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=254489960&repo=autoland&lineNumber=274

Backout: https://hg.mozilla.org/integration/autoland/rev/9924e69c0456033138fb75e2b1e5ba61ce6c980c

Comment 6

[Andreea Pavel [:apavel]]

There is also the following bc failure: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=254493480&repo=autoland&lineNumber=12310

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fa127ef3d04
CustomElement for a modal dialog and add 'Delete' warning dialog r=fluent-reviewers,Pike

Comment 8

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/8fa127ef3d04