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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Add 'ref' for primitives for direct access to Node #139

Merged
merged 7 commits into from Dec 29, 2018

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Dec 22, 2018

This change proposes adding a ref to the primitive nodes, that gives access to the underlying node. This isn't meant to be a common scenario by any means, but there are a few reasons I think we need this:

  • For a control like <Slider />, we need to know the calculated layout values (the width/height) of the component. We can get that directly via the node, but perhaps a better event would be useful for that down the road - like onLayoutChanged?
  • For a widget like <Clickable />, we really want to capture the mouse input at the start of the mousedown gesture, and then, when it is released, check if the mouseup occurred in the bounds of the widget. We can use the hitTest method on the node for this.
  • For focus management - we need some underlying handle to call focus on - I think the node is a natural choice

I'm open to other ideas for this (exposing ref always feels dirty 馃槃 ), but this can at least help us move forward on some of these other UI widgets.

Copy link
Collaborator

@OhadRau OhadRau left a comment

Choose a reason for hiding this comment

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

Everything here looks pretty good to me. The one question I have is whether we want to use the term id here, as this could cause some confusion to people coming from HTML/if we want to use external stylesheets in the future. Apart from that, this is good to go in my opinion.

@bryphe
Copy link
Member Author

bryphe commented Dec 29, 2018

Thanks for the review, @OhadRau ! 馃帀

The one question I have is whether we want to use the term id here, as this could cause some confusion to people coming from HTML/if we want to use external stylesheets in the future.

Good point, I didn't think about this - I could see confusion. Do you have any alternatives? The only thing I can think of is perhaps making it uniqueId, but I'm not sure if that is better?

@OhadRau
Copy link
Collaborator

OhadRau commented Dec 29, 2018

Maybe something like internalId or nodeId? Can't really think of anything that makes it clear what this is either.

@bryphe
Copy link
Member Author

bryphe commented Dec 29, 2018

Thanks for the suggestions, @OhadRau ! Went with internalId 馃憤

@bryphe bryphe merged commit 43ef2bc into master Dec 29, 2018
@bryphe bryphe deleted the bryphe/primitives/ref-api branch December 29, 2018 20:02
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.

None yet

2 participants