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.

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

Picker/Autofill: fixes several minor bugs. #4569

Merged
merged 14 commits into from
Apr 25, 2018

Conversation

joschect
Copy link
Contributor

Pull request checklist

Description of changes

Fixes the following bugs:

  1. BasePicker doesn't call it's own onblur props
  2. BasePicker doesn't call inputBlur when input is actually not focused.
  3. Autofill doesn't respect default display value
  4. Generic item is incorrectly typed.

Focus areas to test

(optional)

@@ -219,4 +219,11 @@ export interface IInputProps extends React.InputHTMLAttributes<HTMLInputElement>
* Screen reader label to apply to an input element.
*/
'aria-label'?: string;
/**
* The default value to be visible when the autofill first created.
* This is different than placeholder text because the palceholder text will disappear. This
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

@@ -444,11 +444,14 @@ export class BasePicker<T, P extends IBasePickerProps<T>> extends BaseComponent<
}

protected onInputBlur = (ev: React.FocusEvent<HTMLInputElement | Autofill>): void => {
// Only blur if an unrelated element gets focus. Otherwise treat it as though it still has focus.
if (this.props.inputProps && this.props.inputProps.onBlur) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in the conditional below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is a callback specifically for the input, there could be a case where something should happen only explicitly when input is blurred. However, the picker should only act when it loses focus entirely.

@joschect joschect closed this Apr 17, 2018
@joschect joschect reopened this Apr 17, 2018
@joschect joschect closed this Apr 17, 2018
@joschect joschect reopened this Apr 17, 2018
@dzearing dzearing closed this Apr 25, 2018
@dzearing dzearing reopened this Apr 25, 2018
@joschect joschect merged commit a59edf9 into microsoft:master Apr 25, 2018
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Apr 26, 2018
* master: (42 commits)
  Applying package updates.
  ProgressIndicator: Finish conversion to mergeStyles (microsoft#4595)
  Fix props validation for Breadcrumb (microsoft#4666)
  No unused vars part of ts (microsoft#4670)
  Picker/Autofill: fixes several minor bugs. (microsoft#4569)
  Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice (microsoft#4662)
  Applying package updates.
  Merge styles order (microsoft#4664)
  Fabric component: revert class change and make it backwards compatible (microsoft#4671)
  Addressing Issue microsoft#3707 - OverflowSet: Add the ability to set aria-label (microsoft#4667)
  Fix input type for Tile ARIA label prop (microsoft#4668)
  Fix theme slots for DetailsList header colors (microsoft#4658)
  Applying package updates.
  Jolore/calendar updates (microsoft#4643)
  Remove wordWrap setting. (microsoft#4657)
  Pivot: convert to mergeStyles - part 1  (microsoft#4656)
  Use the `data-is-scrollable` attribute on the correct ScrollablePane div (microsoft#4602)
  Applying package updates.
  Remove unused iconClassName prop from Nav.types (microsoft#4634)
  Jest snapshots: classes in animations should autoexpand. (microsoft#4647)
  ...
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Apr 26, 2018
* master: (34 commits)
  Applying package updates.
  ProgressIndicator: Finish conversion to mergeStyles (microsoft#4595)
  Fix props validation for Breadcrumb (microsoft#4666)
  No unused vars part of ts (microsoft#4670)
  Picker/Autofill: fixes several minor bugs. (microsoft#4569)
  Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice (microsoft#4662)
  Applying package updates.
  Merge styles order (microsoft#4664)
  Fabric component: revert class change and make it backwards compatible (microsoft#4671)
  Addressing Issue microsoft#3707 - OverflowSet: Add the ability to set aria-label (microsoft#4667)
  Fix input type for Tile ARIA label prop (microsoft#4668)
  Fix theme slots for DetailsList header colors (microsoft#4658)
  Applying package updates.
  Jolore/calendar updates (microsoft#4643)
  Remove wordWrap setting. (microsoft#4657)
  Pivot: convert to mergeStyles - part 1  (microsoft#4656)
  Use the `data-is-scrollable` attribute on the correct ScrollablePane div (microsoft#4602)
  Applying package updates.
  Remove unused iconClassName prop from Nav.types (microsoft#4634)
  Jest snapshots: classes in animations should autoexpand. (microsoft#4647)
  ...
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Getting onblur of tagpicker even when focus is going to tag-suggestion callout from tag-input box
3 participants