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

[TextField] Add the event argument to the onBlur prop #6143

Merged
merged 1 commit into from Jun 15, 2022

Conversation

chloerice
Copy link
Member

WHY are these changes introduced?

Fixes #6115

WHAT is this pull request doing?

Callbacks set on the TextField onBlur prop had to hack around the lack of support for the event argument passed implicitly by the input prior to shipping #5957. This PR fixes regressions upstream in web by adding and passing the event argument to the onBlur prop so that consuming apps don't need to hack their callback's types when composing complex inputs like DatePickerTextField.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

@chloerice chloerice force-pushed the fix-textfield-onblur branch 2 times, most recently from 092ab74 to 8756c99 Compare June 15, 2022 17:37
onFocus?: (event?: React.FocusEvent<HTMLElement>) => void;
/** Callback fired when focus is removed */
onBlur?(): void;
onFocus?: (event?: React.FocusEvent) => void;

Choose a reason for hiding this comment

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

I know it's not technically part of this PR but wondering why the onFocus has a optional event in the parameters. Shouldn't it always be there?

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, good call I'm not sure 🤔

@chloerice chloerice self-assigned this Jun 15, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 15, 2022

size-limit report 📦

Path Size
polaris-react-cjs 196.77 KB (-0.01% 🔽)
polaris-react-esm 132.09 KB (+0.01% 🔺)
polaris-react-esnext 187.8 KB (0%)
polaris-react-css 42.2 KB (0%)

Copy link
Contributor

@Germanika Germanika left a comment

Choose a reason for hiding this comment

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

lgtm! Thanks for the quick fix @chloerice!

Copy link

@melnikov-s melnikov-s left a comment

Choose a reason for hiding this comment

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

🎉

@chloerice chloerice merged commit 20dc2ab into main Jun 15, 2022
@chloerice chloerice deleted the fix-textfield-onblur branch June 15, 2022 19:16
@github-actions github-actions bot mentioned this pull request Jun 15, 2022
chloerice added a commit that referenced this pull request Jun 17, 2022
…#6224)

* [TextField] Reverts onFocus event arg not being optional

* [TextField] Make onBlur event optional
@github-actions github-actions bot mentioned this pull request Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue getting relatedTarget from onBlur via TextField
3 participants