Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Alert Dialog Example #688

Merged
merged 32 commits into from
Jun 24, 2018
Merged

Add Alert Dialog Example #688

merged 32 commits into from
Jun 24, 2018

Conversation

sh0ji
Copy link
Contributor

@sh0ji sh0ji commented May 22, 2018 •
edited by pr-preview bot
Loading

Addresses #101.

I originally had this in the alert folder, but it primarily uses code from the dialog example, so I moved it there. Since it alters some of the code from the dialog example, we'll need to review that as well.

Overview

  • It is a "Confirmation" dialog used to confirm whether the user wants to take a destructive action.
  • Defaults to "no" by placing that button first in the alertdialog (least destructive action).
  • Confirming ("yes") will remove the contents in the <textarea>.
  • Pressing the "discard contents" call to action when there are no contents triggers an alert. I chose to do this to illustrate the difference between an alert and an alertdialog, but it could easily be removed if it's out of scope.

Changes

I had to change some pieces of the dialog.js and dialog.css used by the dialog-modal example in order to accommodate this new example. Those changes are documented here.

  • Changes the way dialogs are positioned to better fit their contents (9b3e274)
  • Added a box-shadow to the dialog element (7ddd9c5)
  • Fixed a sentence fragment in dialog.html (3c92514)
  • Adds alertdialog to the role validator in the Dialog constructor (6de2fd0)

b64883f changes a few things:

  • Removed all body/#base_window_layer CSS from dialog.css.
    • These declarations tied dialog code very tightly to the document structure, which was causing issues in the new alertdialog example.
    • This was also overriding the default body layout defined by base.css, which was actually causing some undocumented layout issues in the dialog-modal example. It can be seen in the current TR.
  • dialog.js now adds a class to the body in order to disable body scroll when a dialog is open. #base_window_layer was previously handling scroll.
  • Layout changes meant that the #base_window_layer was no longer doing anything, so I removed it from dialog.html

Rawgit Links

Preview | Diff

sh0ji added 10 commits May 21, 2018 14:21
@sh0ji
make room for alertdialog.html
6223a21
@sh0ji
initial alertdialog
07ba6d6
@sh0ji
editorial: fix fragment sentence
3c92514
@sh0ji
target labels directly
ac5502f
@sh0ji
improve dialog sizing & positioning for desktop
9b3e274 
changes the design to mobile-first, and allows the size of the modal to
better fit its contents on desktop
@sh0ji
add box shadow
7ddd9c5
@sh0ji
move to dialog-modal folder
c585cca
@sh0ji
add alertdialog to the list of valid Dialog roles
6de2fd0
@sh0ji
make the alert notification more prominent
704e4db
@sh0ji
remove #base_window_layer
b64883f 
This layer was previously overriding the layout defined by
https://www.w3.org/StyleSheets/TR/2016/base.css, as well as acting as a
scroll container for the non-dialog content. It was removed in favor of
the base layout, and scroll is simply disabled on the body when a dialog
is open.
@mcking65 mcking65 self-assigned this Jun 1, 2018
sh0ji added 19 commits June 1, 2018 16:31
@sh0ji
make room for alertdialog.html
2300da9
@sh0ji
initial alertdialog
e3fb535
@sh0ji
editorial: fix fragment sentence
4228323
@sh0ji
target labels directly
917678f
@sh0ji
improve dialog sizing & positioning for desktop
6366416 
changes the design to mobile-first, and allows the size of the modal to
better fit its contents on desktop
@sh0ji
add box shadow
e72cb94
@sh0ji
move to dialog-modal folder
1c5996f
@sh0ji
add alertdialog to the list of valid Dialog roles
08adb33
@sh0ji
make the alert notification more prominent
7fb3064
@sh0ji
remove #base_window_layer
7ac78fb 
This layer was previously overriding the layout defined by
https://www.w3.org/StyleSheets/TR/2016/base.css, as well as acting as a
scroll container for the non-dialog content. It was removed in favor of
the base layout, and scroll is simply disabled on the body when a dialog
is open.
@sh0ji
clicking and selecting are different
ea4e5d5
@sh0ji
be more consistent about gt/lt usage
b2692e0
@sh0ji
vnu: The “type” attribute is unnecessary for JavaScript resources.
0ba27b0
@sh0ji
serialize the interaction features for easier readability
d61ceb0
@sh0ji
more consistent code highlighting
312fcc7
@sh0ji
add a space
f88cf4d
@sh0ji
formatting
9cc53ae
@sh0ji
more consistent language
239696f
@sh0ji
Merge branch 'alertdialog-example' of https://github.com/sh0ji/aria-p…
701c067 
…ractices into alertdialog-example
@sh0ji
Copy link
Contributor Author

sh0ji commented Jun 1, 2018

Rebased and fixed a bunch of editorial issues. No functional changes.

@mcking65 mcking65 mentioned this pull request Jun 13, 2018
Closed
@mcking65 mcking65 changed the title Alertdialog example Add Alert Dialog Example Jun 14, 2018
@mcking65
Copy link
Contributor

@sh0ji, could you use the edit buttton on this PR and change the base branch from master to alertdialog?

@sh0ji sh0ji changed the base branch from master to alertdialog June 24, 2018 02:49
@mcking65
Copy link
Contributor

mcking65 commented Jun 24, 2018 •
edited
Loading

Thanks @sh0ji, merging now.

@mcking65 mcking65 merged commit 6f4eb22 into w3c:alertdialog Jun 24, 2018
@sh0ji sh0ji deleted the alertdialog-example branch November 12, 2018 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mcking65 mcking65

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sh0ji @mcking65