New issue
Advanced search Search tips

Issue 843868 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: 2
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 841281
issue 3598



Sign in to add a comment

Create chops-dialog component or find another replacement for paper-dialog

Project Member Reported by zhangtiff@chromium.org, May 17 2018

Issue description

I've had several issues with using Polymer's paper-dialog component, including: 

- Depending on its parent, the modal appears behind the overlay at times, rendering it unusuable in these cases. This issue is marked as WontFix on paper-dialog's end: https://github.com/PolymerElements/paper-dialog/issues/152 
- By default paper-dialog doesn't resize when its child content resizes. I don't know if this behavior is intentional on their end but in most cases, this behavior feels unexpected. 
- crbug.com/841281

I'd like to create our own dialog/modal element for use in Chops. We could build this on top of https://www.webcomponents.org/element/PolymerElements/iron-overlay-behavior or we could make our own overlay behavior. I'll have to dig a bit more into this. 
 
Blocking: 841281 3598
Cc: seanmccullough@chromium.org
Components: Infra>UI
Labels: -Pri-3 Pri-1
Owner: zhangtiff@chromium.org
Status: Assigned (was: Untriaged)
Summary: Create chops-dialog component or find another replacement for paper-dialog (was: Create chops-dialog component)
Raising the priority on this because this is blocking both a bugfix for SoM and frontend work on FLT (I ran into some of these bugs while working on that). I want to fix this by early next week. 
When I work on this, I want to refer to this post for accessibility: https://bitsofco.de/accessible-modal-dialog/
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6

commit 92c71d41e4f66a3ea4abd34eae22e3a8389c73a6
Author: Tiff Zhang <zhangtiff@google.com>
Date: Sat Jun 02 00:10:09 2018

ChopsUI: Add chops-dialog.

Unfortunately, because of  crbug.com/848595  I can't demo the elements in chopsui-gallery at the moment.

But I did develop these testing them on the attachment modals on Monorail, so you can see a demo there.

Demo: https://15745-52a7db3-tainted-zhangtiff-dot-monorail-staging.appspot.com/p/chromium/issues/approval?id=1&you=ux

(Click on the attachment images to demo.)

I tried a few different approaches here before deciding to go down the path of implementing our own dialog.

This dialog does have a notable tradeoff that separates it from paper-dialog/iron-overlay-behavior, which is that it doesn't expose the main dialog box as available for styling through the :host. I made this call because I wasn't able to other include an overlay
without running into https://github.com/PolymerElements/paper-dialog/issues/152

Otherwise, the API is a lot lighter than paper-dialog but mostly in the same vein.

For accessibility, I implemented the requirements listed in: https://bitsofco.de/accessible-modal-dialog/ I


Bug:843868
Change-Id: Id8b8e37db1ed108879b937e673c0e0699db5aeb7
Reviewed-on: https://chromium-review.googlesource.com/1081936
Commit-Queue: Tiffany Zhang <zhangtiff@chromium.org>
Reviewed-by: Sean McCullough <seanmccullough@chromium.org>

[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/appengine/monorail/elements/flt/mr-issue-details/mr-attachment-gallery.html
[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/appengine/monorail/README.md
[add] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/chops-dialog.html
[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/appengine/monorail/elements/flt/mr-issue-details/mr-attachment-gallery.js
[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/test/chops-carousel_test.html
[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/README.md
[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/chops-button.html
[modify] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/chops-carousel.html
[add] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/demo/chops-dialog_demo.html
[add] https://crrev.com/92c71d41e4f66a3ea4abd34eae22e3a8389c73a6/crdx/chopsui/test/chops-dialog_test.html

Sign in to add a comment