-
Notifications
You must be signed in to change notification settings - Fork 352
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.
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
Combobox Examples with listbox popup: Fix Escape behavior, mouse interaction, visual design, and more #1276
Changes from 27 commits
f9f0199
3663dea
db05fa5
7937a03
d9464c8
903f6d3
60e8955
2468850
98b0967
5235675
245d30e
565a0c1
98f6d55
4b8524f
2abf21c
670bb24
6a41816
f6d6b32
faedb8c
11bdddd
0d352f4
8f303a1
22575ec
710f769
08150b9
ad47b9d
4a00ac8
c373245
9bccd49
ef94e2f
58dd2a0
8edcb40
8577088
03ee3f2
965a45c
ad2d9ef
2125c15
511a707
4e42a82
7554162
9614aff
28f0ebd
5be02da
2a5719e
baf2bed
ff804c1
9066a96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
margin: 0; | ||
vertical-align: bottom; | ||
border: 1px solid gray; | ||
position: relative; | ||
} | ||
|
||
.combobox .group input { | ||
|
@@ -30,14 +31,33 @@ | |
width: 1.25rem; | ||
border-left: none; | ||
outline: none; | ||
font-size: 70%; | ||
} | ||
|
||
.combobox .group button.open svg { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non blocking comment: it'd be nice to keep CSS selectors to a max of three levels deep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a common best practice for efficiency (browser go right-to-left in interpreting CSS selectors), and to avoid an arms race of over-specific CSS selectors. for reference: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, thanks. For efficiency, I think it's better to use child selector over descendant selector, but that would add even more specificity and result in harder to read selectors. I think the selectors here can remove some middle part without changing what matches, so I'll try that. |
||
transform: rotate(180deg) translate(0, -1px); | ||
} | ||
|
||
.combobox .group.focus { | ||
outline: 2px solid black; | ||
outline-offset: 1px; | ||
} | ||
|
||
.combobox .group.focus input, | ||
.combobox .group.focus button { | ||
background-color: #DEF; | ||
} | ||
|
||
.combobox .group svg polygon { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above, three levels max would be nice |
||
fill: gray; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change this to |
||
stroke: gray; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change this to |
||
} | ||
|
||
.combobox button.open svg polygon, | ||
.combobox .group.focus svg polygon { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here as well |
||
fill: black; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change this to |
||
stroke: black; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change this to |
||
} | ||
|
||
ul[role="listbox"] { | ||
margin: 0; | ||
padding: 0; | ||
|
@@ -50,26 +70,29 @@ ul[role="listbox"] { | |
box-sizing: border-box; | ||
border: 1px gray solid; | ||
border-top: none; | ||
max-height: 12em; | ||
max-height: 11.4em; | ||
width: 12rem; | ||
overflow: scroll; | ||
overflow-x: hidden; | ||
font-size: 87.5%; | ||
} | ||
|
||
ul[role="listbox"] li[role="option"] { | ||
border-top: 1px solid transparent; | ||
border-bottom: 1px solid transparent; | ||
display: block; | ||
padding-left: 0.3em; | ||
padding-top: 0.2em; | ||
padding-bottom: 0.2em; | ||
margin: 0; | ||
padding: 0.1em 0.3em; | ||
} | ||
|
||
/* focus and hover styling */ | ||
|
||
[role="listbox"].focus [role="option"][aria-selected="true"] { | ||
background-color: #DEF; | ||
border-color: #8CCBF2; | ||
padding-top: 0; | ||
padding-bottom: 0; | ||
border-top: 0.2em solid #8CCBF2; | ||
border-bottom: 0.2em solid #8CCBF2; | ||
} | ||
|
||
@media (forced-colors: active), (-ms-high-contrast: active) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be worth adding
aria-hidden="true"
on here and the other examples, or switching to an svgtitle
method of labelling +role="img"
, to prevent navigation into the SVG, though it's a pretty minor thingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I also added
focusable="false"
per #1045 (comment) -- though, possibly this SVG should be in the CSS instead, with generated content?