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

chore: small improvements to shared ref handler logic #4475

Merged
merged 6 commits into from
Jan 11, 2021

Conversation

adidahiya
Copy link
Contributor

Follow-up to #4438

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Add code documentation for setRef and refHandler utilities
  • Improve ergonomics of refHandler utility, since the ref prop can be optional
  • Improve type inference of Button and AnchorButton ref handlers by using conditional type in AbstractButton interface
  • Remove unnecessary IRefHandlerContainer interface, just list the handlers as separate members in a few places

Reviewers should focus on:

Screenshot

public static displayName = `${DISPLAYNAME_PREFIX}.Button`;

// need to keep this ref so that we can access it in AbstractButton#handleKeyUp
public buttonRef: HTMLButtonElement | IRefObject<HTMLButtonElement> | null = null;

protected handleRef: IRef<HTMLButtonElement> = refHandler(this.props.elementRef, this, "buttonRef");
protected handleRef: IRef<HTMLButtonElement> = refHandler(this, "buttonRef", this.props.elementRef);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before this PR, the first generic type param of refHandler was being inferred as any. now it's HTMLButtonElement.

@blueprint-bot
Copy link

Fix regression which made IButtonProps type param required

Previews: documentation | landing | table

@blueprint-bot
Copy link

Fix test compilation

Previews: documentation | landing | table

@adidahiya adidahiya merged commit f2ffb6f into develop Jan 11, 2021
@adidahiya adidahiya deleted the ad/cleanup-ref-handlers branch January 11, 2021 20:43
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.

2 participants