1693277 - Implement new modal dialog styling

Status: RESOLVED FIXED

(Toolkit Graveyard :: Notifications and Alerts, enhancement, P1)

Description

[:Gijs (he/him)]

(Figma is leading here, but I've tried to extract salient details.)

• website modals default to 412px wide (should probably set in em so that we scale up if the user has custom text sizes), shrink if there's no room;
• Firefox modals default to 412px but we make the bookmarks dialog wider
• height maxes out at viewport height -10% (5 at the top, 5 at the bottom)
• long wrapped messages overflow with a fade-out at the bottom
• Break words for text that would overflow horizontally
• 8px border radius
• box shadow of 0px 2px 14px rgba(58, 57, 68, 0.2);
• 16px space from the edge to all the content (top to title, start to text / checkbox, bottom and end to buttons)
• bold font for the header / title for Firefox dialogs (x-ref bug 1693008)
• standard background and button colours / components ("large" buttons, so the 8px variants)

Comment 1

[Micah [:mtigley] (she/her)]

(In reply to :Gijs (he/him) from comment #0)

• 8px border radius

The common dialog is opened using window.openDialog. Do we have existing styles somewhere for in-content windows?

Comment 2

[:Gijs (he/him)]

(In reply to Micah Tigley [:mtigley] from comment #1)

(In reply to :Gijs (he/him) from comment #0)

• 8px border radius

The common dialog is opened using window.openDialog. Do we have existing styles somewhere for in-content windows?

We'll load the in-content/common.css stylesheet via the SubDialog.jsm code. It looks like this might be messy to make work, because the shadow for the dialog is currently generated by the frame, and the background colour is set inside it - so either we need at least 8px of the padding to be on the frame and give it a background colour and the border radius and th ebox-shadow, or we need to move the radius'd border and the box shadow inside the frame (making everything else transparent). I haven't really checked which of these would be easier.

It's probably worth splitting this bug up a bit more...

Comment 3

[:Gijs (he/him)]

Actually, it seems the box-shadow is on the dialogBox element that contains the browser, and we could just give that a border-radius, clipping the contents (including the browser element and its contents). This would only be an issue if we then tried to display a scrollbar inside the dialog, flush with the right-hand-side -- but the title and buttons will want to be permanently visible anyway, so the scrollbars shouldn't make it to the curved bits in the first place, so that should be OK.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

As per guidance from Vicky, for tracking, we're marking all the bugs that people are working on as P1.

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1693277 - fix long text wrapping, shadows, radii and colours for proton, and clean up some CSS, r?mtigley

This moves some inline styles into CSS and fixes modal masks and shadows to match the spec.

I also noticed some negative effects from other Proton button styles on close-icon buttons in dialogs
in about:preferences (e.g. check the oversized titlebar for the fonts dialog) that I fixed here.

Comment 6

[:Gijs (he/him)]

Attachment: Bug 1693277 - fix positioning of content dialogs irrespective of size, r?mtigley

This commit uses CSS grid layout to position content modal prompts, and to get the
requisite 5% top and bottom margins in a way that doesn't require JS to update,
and adapts to resizing and things like the find bar and devtools opening. To make
this work right it also removes the 5px negative top margin for these dialogs.

Then this commit adds some logic to SubDialog.jsm to support this behaviour.
Prior to this change, SubDialog.jsm sets height/width on the dialogs that go
through it, except if they pass sizeto=available (used by the print dialog).

This new sizeTo value similarly avoids all the complex sizing logic - but also
avoids the print logic of having an aspect ratio to maintain when the window
changes size. We use the content size it determines to set the height of the
dialog (a grid row): either 90% (so there's 5% above and 5% below) or the
document height, whichever is smaller.

The next commit will use this setup to deal with the problem of variable length
content inside the dialog that we're trying to show.

Depends on D107109

Comment 7

[:Gijs (he/him)]

Attachment: Bug 1693277 - fix sizing of dialogs with various content sizes, r?mtigley

This uses the new sizeTo value supported by SubDialog.jsm to also impact
how we size things inside the dialog. It's a bit complicated because we
need the dialog to support 3 layouts:

1. the window-modal setup (which we're trying not to touch)
2. the non-proton modal setup (which uses grid layout to have one column with
   the image and the username/password fields, where applicable)
3. the proton modal setup (where we don't want the grid layout, as
   the labels for username/password go above their text fields and there's no
   big icon on the left, so there's no point)

Finally, for content modal prompts, we need to effectively determine the size
of the content and whether we need to overflow. We can't just always allow
flexbox to shrink everything, or we end up with overflow even for dialogs
with just 2 lines of text.

So with this patch, we determine the ideal height of the dialog document from
SubDialog.jsm, before setting the .sizeDetermined class there, thus allowing
the #infoBody part to overflow. The SubDialog.jsm code will ensure the
embedding browser is large enough to allow not overflowing/scrolling the
content.

In terms of the 3 layouts, we select for "not 1" by using dialog[subdialog]
(an attribute set by the SubDialog.jsm code anyway). We then use proton
pref-based @supports queries to pick between layouts (2) and (3).

Depends on D107110

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b9986d057d7
fix long text wrapping, shadows, radii and colours for proton, and clean up some CSS, r=mtigley
https://hg.mozilla.org/integration/autoland/rev/be8108cd9820
fix positioning of content dialogs irrespective of size, r=mtigley
https://hg.mozilla.org/integration/autoland/rev/0580aaec32a0
fix sizing of dialogs with various content sizes, r=mtigley

Comment 9

[Alexandru Michis [:malexandru]]

Backed out 3 changesets (bug 1693277) for causing bc failures in browser_modal_resize.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/39b7de93a5d87500f9ab9a48088b48d10b2eccd8
Push with failures, failure log.

Comment 10

[:Gijs (he/him)]

(In reply to Alexandru Michis [:malexandru] from comment #9)

Backed out 3 changesets (bug 1693277) for causing bc failures in browser_modal_resize.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/39b7de93a5d87500f9ab9a48088b48d10b2eccd8
Push with failures, failure log.

The test correctly determined (even if the messaging was somewhat obfuscated) that I broke the 5px overlap specifically for the print dialog. I'll fix it and re-push.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/acef2338bb0a
fix long text wrapping, shadows, radii and colours for proton, and clean up some CSS, r=mtigley
https://hg.mozilla.org/integration/autoland/rev/78d07988e76a
fix positioning of content dialogs irrespective of size, r=mtigley
https://hg.mozilla.org/integration/autoland/rev/0fc57efb08a0
fix sizing of dialogs with various content sizes, r=mtigley

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/acef2338bb0a
https://hg.mozilla.org/mozilla-central/rev/78d07988e76a
https://hg.mozilla.org/mozilla-central/rev/0fc57efb08a0