-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix(pickers) docs & styles for non-layered dropdowns for mobile a11y #18495
Changes from all commits
4d2eeb8
f2f45c0
7b43ee3
2d3a06a
164b437
c12a266
611dbe3
4ddd384
15af596
92705fa
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 |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "minor", | ||
"comment": "fix zIndex style for Callouts with doNotLayer=true", | ||
"packageName": "@fluentui/react", | ||
"email": "sarah.higley@microsoft.com", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "patch", | ||
"comment": "Add picker example of inline suggestions dropdown", | ||
"packageName": "@fluentui/react-examples", | ||
"email": "sarah.higley@microsoft.com", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
import * as React from 'react'; | ||
|
||
import { TagPicker, ITag, IBasePickerSuggestionsProps } from '@fluentui/react/lib/Pickers'; | ||
import { mergeStyles } from '@fluentui/react/lib/Styling'; | ||
import { useId } from '@fluentui/react-hooks'; | ||
|
||
const rootClass = mergeStyles({ | ||
maxWidth: 500, | ||
}); | ||
|
||
const pickerSuggestionsProps: IBasePickerSuggestionsProps = { | ||
suggestionsHeaderText: 'Suggested colors', | ||
noResultsFoundText: 'No color tags found', | ||
}; | ||
|
||
const testTags: ITag[] = [ | ||
'black', | ||
'blue', | ||
'brown', | ||
'cyan', | ||
'green', | ||
'magenta', | ||
'mauve', | ||
'orange', | ||
'pink', | ||
'purple', | ||
'red', | ||
'rose', | ||
'violet', | ||
'white', | ||
'yellow', | ||
].map(item => ({ key: item, name: item[0].toUpperCase() + item.slice(1) })); | ||
|
||
const listContainsTagList = (tag: ITag, tagList?: ITag[]) => { | ||
if (!tagList || !tagList.length || tagList.length === 0) { | ||
return false; | ||
} | ||
return tagList.some(compareTag => compareTag.key === tag.key); | ||
}; | ||
|
||
const filterSuggestedTags = (filterText: string, tagList: ITag[]): ITag[] => { | ||
return filterText | ||
? testTags.filter( | ||
tag => tag.name.toLowerCase().indexOf(filterText.toLowerCase()) === 0 && !listContainsTagList(tag, tagList), | ||
) | ||
: []; | ||
}; | ||
|
||
const getTextFromItem = (item: ITag) => item.name; | ||
|
||
export const TagPickerInlineExample: React.FunctionComponent = () => { | ||
const pickerId = useId('inline-picker'); | ||
|
||
return ( | ||
<div className={rootClass}> | ||
<label htmlFor={pickerId}>Choose a color</label> | ||
<TagPicker | ||
removeButtonAriaLabel="Remove" | ||
selectionAriaLabel="Selected colors" | ||
onResolveSuggestions={filterSuggestedTags} | ||
getTextFromItem={getTextFromItem} | ||
pickerSuggestionsProps={pickerSuggestionsProps} | ||
itemLimit={4} | ||
// this option tells the picker's callout to render inline instead of in a new layer | ||
pickerCalloutProps={{ doNotLayer: true }} | ||
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. To make it more explicit, can you add a comment calling out that this is the important part? 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. Done! Let me know if you like the wording. |
||
inputProps={{ | ||
id: pickerId, | ||
}} | ||
/> | ||
</div> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`Component Examples renders TagPicker.Inline.Example.tsx correctly 1`] = ` | ||
<div | ||
className= | ||
|
||
{ | ||
max-width: 500px; | ||
} | ||
> | ||
<label | ||
htmlFor="inline-picker0" | ||
> | ||
Choose a color | ||
</label> | ||
<div | ||
className="ms-BasePicker" | ||
onBlur={[Function]} | ||
onFocus={[Function]} | ||
onKeyDown={[Function]} | ||
> | ||
<span | ||
hidden={true} | ||
id="selected-items-id__1-label" | ||
> | ||
Selected colors | ||
</span> | ||
<div | ||
className="ms-SelectionZone" | ||
onClick={[Function]} | ||
onContextMenu={[Function]} | ||
onDoubleClick={[Function]} | ||
onFocusCapture={[Function]} | ||
onKeyDown={[Function]} | ||
onKeyDownCapture={[Function]} | ||
onMouseDown={[Function]} | ||
onMouseDownCapture={[Function]} | ||
role="presentation" | ||
> | ||
<div | ||
className= | ||
ms-BasePicker-text | ||
{ | ||
align-items: center; | ||
border-radius: 2px; | ||
border: 1px solid #605e5c; | ||
box-sizing: border-box; | ||
display: flex; | ||
flex-wrap: wrap; | ||
min-height: 30px; | ||
min-width: 180px; | ||
position: relative; | ||
} | ||
&:hover { | ||
border-color: #323130; | ||
} | ||
> | ||
<input | ||
aria-autocomplete="both" | ||
aria-controls="" | ||
aria-expanded={false} | ||
aria-haspopup="listbox" | ||
autoCapitalize="off" | ||
autoComplete="off" | ||
className= | ||
ms-BasePicker-input | ||
{ | ||
-moz-osx-font-smoothing: grayscale; | ||
-webkit-font-smoothing: antialiased; | ||
align-self: flex-end; | ||
background-color: transparent; | ||
border-radius: 2px; | ||
border: none; | ||
color: #323130; | ||
flex-grow: 1; | ||
font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif; | ||
font-size: 14px; | ||
font-weight: 400; | ||
height: 30px; | ||
outline: none; | ||
padding-bottom: 0; | ||
padding-left: 6px; | ||
padding-right: 6px; | ||
padding-top: 0; | ||
} | ||
&::-ms-clear { | ||
display: none; | ||
} | ||
data-lpignore={true} | ||
id="inline-picker0" | ||
onBlur={[Function]} | ||
onChange={[Function]} | ||
onClick={[Function]} | ||
onCompositionEnd={[Function]} | ||
onCompositionStart={[Function]} | ||
onCompositionUpdate={[Function]} | ||
onFocus={[Function]} | ||
onInput={[Function]} | ||
onKeyDown={[Function]} | ||
role="combobox" | ||
spellCheck={false} | ||
style={ | ||
Object { | ||
"fontFamily": "inherit", | ||
} | ||
} | ||
value="" | ||
/> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,12 @@ describe('BasePicker', () => { | |
afterEach(() => { | ||
ReactDOM.unmountComponentAtNode(root); | ||
document.body.textContent = ''; | ||
|
||
// reset any jest timers | ||
if ((setTimeout as any).mock) { | ||
jest.runOnlyPendingTimers(); | ||
jest.useRealTimers(); | ||
} | ||
}); | ||
|
||
const BasePickerWithType = BasePicker as new (props: IBasePickerProps<ISimple>) => BasePicker< | ||
|
@@ -110,6 +116,32 @@ describe('BasePicker', () => { | |
disabledTests: ['component-has-root-ref', 'component-handles-ref', 'has-top-level-file'], | ||
}); | ||
|
||
it('renders inline callout', () => { | ||
jest.useFakeTimers(); | ||
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. Can you add cleanup for this in
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. done :) |
||
document.body.appendChild(root); | ||
const picker = React.createRef<IBasePicker<ISimple>>(); | ||
|
||
ReactDOM.render( | ||
<BasePickerWithType | ||
onResolveSuggestions={onResolveSuggestions} | ||
onRenderItem={onRenderItem} | ||
onRenderSuggestionsItem={basicSuggestionRenderer} | ||
componentRef={picker} | ||
pickerCalloutProps={{ doNotLayer: true, id: 'test' }} | ||
/>, | ||
root, | ||
); | ||
|
||
const input = document.querySelector('.ms-BasePicker-input') as HTMLInputElement; | ||
input.focus(); | ||
input.value = 'b'; | ||
ReactTestUtils.Simulate.input(input); | ||
runAllTimers(); | ||
|
||
const calloutParent = document.getElementById('test')?.closest('.ms-BasePicker'); | ||
expect(calloutParent).toBeTruthy(); | ||
}); | ||
|
||
it('can provide custom renderers', () => { | ||
jest.useFakeTimers(); | ||
document.body.appendChild(root); | ||
|
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.
Super nitpicky/opinion-based but I think it looks a bit funny to have the prop on its own line (up to you though).
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.
This is the only comment I didn't change -- I'm leaning towards keeping it on a new line, just because I'm really hoping people will actually use it, so I'd like to make it visually called out & easy to copy/paste. I'm not super tied to it though -- in vNext, I think I'd actually like it to be inline by default, with a prop to render it in a new layer 😄