Batch cherry-pick of react-example fixes #16677
Conversation
…st example (#16497) #### Pull request checklist - [x] Addresses an existing issue: Fixes microsoftdesign/fluentui#10836 - [x] Include a change request file using `$ yarn change`: No change files are needed #### Description of changes Remove `display: block` from the `key` column links and add an `href=#` to properly mark the `name` column links as hyperlinks.
…#16496) #### Pull request checklist - [x] Addresses an existing issue: Fixes microsoftdesign/fluentui#10301 - [x] Include a change request file using `$ yarn change`: No change files are needed #### Description of changes Add `title` to input describing the allowed characters in the `Text Field Basic` example
…xample (#16494) #### Pull request checklist - [x] Addresses an existing issue: Fixes microsoftdesign/fluentui#10282 - [x] Include a change request file using `$ yarn change`: No change files are needed #### Description of changes Add descriptive names for the icons used in the OverflowSet Vertical example. Removed unused `icon` props for the `overflowItems`
…ile examples (#16479) - [x] Addresses an existing issue: Fixes microsoftdesign/fluentui#10275 - [x] Include a change request file using `$ yarn change`: No change files are needed Added Announced for the adding person action Changed button label to improve clarity
…16475) * fix(react-examples): Add tooltips to DetailsList documents example (microsoftdesign/fluentui#10255) * Removed extraneous id from tooltip
msft-fluent-ui-bot
added
the
needs cherry-pick
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit de55066:
|
) * fix(react-examples): Add title prop to icons in OverflowSet example (microsoftdesign/fluentui#10284) * Replace title prop for TooltipHost * fix(react-examples): Change item names to match icon in OverflowSet example (#16494) - [x] Addresses an existing issue: Fixes microsoftdesign/fluentui#10282 - [x] Include a change request file using `$ yarn change`: No change files are needed Add descriptive names for the icons used in the OverflowSet Vertical example. Removed unused `icon` props for the `overflowItems` * Update snapshots * revert snapshots * Update just one snapshot * Remove extraneous id on tooltips Add label to overflow button Co-authored-by: Elizabeth Craig <[email protected]>
andrefcdias
removed
the
needs cherry-pick
|
@ecraig12345, regarding the 2 PRs on the Date examples (#16430 and #16463) the files in office-ui-fabric-react/DatePicker seem very different from the files in @fluentui/react/DatePicker. How should the approach be for these cases? |
Perf AnalysisNo significant results to display. All results
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 65db403ef1748737b86370e47e1ef8cd40574f51 (build) |
@andrefcdias Unfortunately you'll have to just look through the react/DatePicker examples manually and determine which ones need the same fix. |
| @@ -722,7 +721,11 @@ export class DetailsListAdvancedExample extends React.Component<{}, IDetailsList | |||
| column.isMultiline = true; | |||
| column.minWidth = 200; | |||
| } else if (column.key === 'name') { | |||
| column.onRender = (item: IExampleItem) => <Link data-selection-invoke={true}>{item.name}</Link>; | |||
| column.onRender = (item: IExampleItem) => ( | |||
| <Link href="#" data-selection-invoke={true}> | |||
There was a problem hiding this comment.
Should have caught this in master, but if you're setting href=# you should probably also add a click handler that does ev.preventDefault() to prevent jumping to the top of the page or other strange actions.
There was a problem hiding this comment.
The href is solely there for it to be a valid anchor and not for the functionality itself. Plus, we would probably need to inform the user why nothing happened when he clicked a link, no?
If you think it's pertinent, let's open a separate issue since the purpose of this PR was solely for cherry-picking into v7.
There was a problem hiding this comment.
I believe it will do the onItemInvoked action on click. Needing to have a click handler with preventDefault is a general issue with using # as href.
There was a problem hiding this comment.
Hmm actually something seems to be preventing the behavior of jumping to the top already, so I guess that's not an issue. Though I see what you mean about the action not being obvious (either to a screen reader user or sighted user) since it just logs to the console...
There was a problem hiding this comment.
How do you want to proceed on this?
There was a problem hiding this comment.
Sorry for not being clear. Let's just leave it as-is.
| @@ -22,7 +22,7 @@ export const TextFieldBasicExample: React.FunctionComponent = () => { | |||
| <TextField label="With error message" errorMessage="Error message" /> | |||
| </Stack> | |||
| <Stack {...columnProps}> | |||
| <MaskedTextField label="With input mask" mask="m\ask: (999) 999 - 9999" /> | |||
| <MaskedTextField label="With input mask" mask="m\ask: (999) 999 - 9999" title="A 10 digit number" /> | |||
There was a problem hiding this comment.
I think I missed this change in master too--what issue was this fixing? Wondering if aria-label (with the label plus instructions) would be more appropriate.
There was a problem hiding this comment.
I had considered that initially, but upon reading the HTML input spec opted for the title solution, as suggested also by @JustSlone.
|
Hello @ecraig12345! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
🎉 Handy links: |
Pull request checklist
$ yarn changeDescription of changes
Aggregate cherry-picking for all my contributions for the
react-examplespackageFocus areas to test