discussions-and-proposals icon indicating copy to clipboard operation
discussions-and-proposals copied to clipboard

Handling platform specific properties

Open kikisaints opened this issue 4 years ago • 16 comments

Introduction

With the nearly full revision of the React Native for Windows project into C++ underway, and the investigations being done there around how it might interface with other related React Native platform implementations (like MacOS), I wanted to start a discussion around how we could or should handle platform-specific properties.

This discussion is related to #50 Unifying the way platforms are handled as well as the newly proposed #126 PlatformColor property - as both are discussions around platform specific objectives; however, I wanted to drill in a bit more to a piece of the conversation from #50 and abstract from #126 to focus on more than just colors.

The Core of It

One thing that the Windows team is currently working on during our conversion to C++, is how to handle certain events that you wouldn't find or necessarily need on an iOS or Android platform - An example of this would be the MouseEntered event that sits on UIElements.

This event doesn't really make sense on Android and iOS, so how do we add it to the View component?

There needs to be a way for events or properties like the MouseEntered api to be handled on platforms where it make sense for it to exist and be ignored on ones where it doesn't.

Looking at @tom-un's proposal for how to handle platform specific colors, it would be interesting if we could look at doing something similar for other properties.

Note: The following pseudo code is meant to be a starting discussion point and not necessarily a proposed solution.

render() {
  const props = Platform.select({
    Windows: {onMouseHoverWindows: this.hover},
    MacOS: {onMouseHoverMacOS: this.hover},
    default: {}
  });

  return (
    <View platformProperty={props}>
      <Text> Example Text </View>
    </View>
  );
}

This is just one approach. There are also other ways of tackling platform properties, as in introducing something like View.WindowsOnMouseEnter, which is similar to how the web has done browser specific functionality. It could be that we start out with something like the above, but one for each platform where it's applicable:

  • View.WindowsOnMouseEnter
  • View.MacOSOnMouseEnter

And then, again similar to the web, if/once it's standardized it could be upgraded to a core property.

Discussion Points

  • What's the best approach to handling platform specific properties or methods in React Native?
  • How can we ensure that the core is still extensible and allows other platforms the flexibility to write code without having to check changes into core?
  • What's the strategy around when platform-specific properties get upgraded to a core property and what's the criteria for meeting that?

kikisaints avatar Jan 16 '20 19:01 kikisaints

Shouldn't the event just be onMouseEnter and no-op'd/ignored on platforms that don't support those events? If you need to further drill down you could do a platform check inside your function, rather than having diverging platform props.

kevinvangelder avatar Jan 17 '20 01:01 kevinvangelder

@Kevinvangelder, one reason to have prefixed props would be because they don’t necessarily have stable consistent behavior and the platforms are figuring out what that looks like first.

———

Do you have a list of props and events in mind that you are trying to figure out what to do with? Your example mentions a couple that I think have different answers. For example, callbacks like mouse event should be on a touchable component, not view.

I’m curious if you have a list because that might help make this more concrete

TheSavior avatar Jan 17 '20 03:01 TheSavior

@TheSavior That makes sense - that the touchable component should handle all input-based events.

As far the the list is concerned, a lot of our platform-specific property implementations are done on an "as needed" basis; however, taking a look at the properties and methods that exist on the UIElement class - I believe that as we continue to work on React Native for Windows it's very likely we'll start needing/wanting to expose more of the properties and events listed there.

A cursory scan of the list, some that pop out to me as ones we may look into, are:

  • GettingFocus
  • GotFocus
  • LosingFocus
  • LostFocus
  • KeyboardAccelerators
  • ProcessKeyboardAccelerators
  • CapturePointer
  • Visibility
  • Drag Events (DragEnter, DragLeave, DragOver)
  • Drop
  • HoldingEvent

Some of these properties are set directly (like Visibility and KeyboardAccelerators) some are events that are listened to/need to be handled.

kikisaints avatar Jan 17 '20 18:01 kikisaints

Focus

  • GettingFocus
  • GotFocus
  • LosingFocus
  • LostFocus

Currently the only thing in React Native that can have focus is TextInput. We know this is wrong but this is how it is right now. Figuring out what to do with these props probably is dependent on figuring out how we want to handle focus overall. An example of why it is important to figure out focus overall is because ScrollResponder has some knowledge about what has focus so that when you tap on the ScrollView it will blur whatever currently has focus. Currently this only works with TextInputs.

Keyboard Shortcuts

  • KeyboardAccelerators
  • ProcessKeyboardAccelerators

It seems like these don't necessarily have to be attached to existing components or modules. It seems like Windows could supply a custom component / native module that adds this behavior. Once that API is figured out and it is applicable to more platforms then that module could become more cross platform and standardized (but maybe still not needing to be in core!)

Drag n' Drop

  • Drag Events (DragEnter, DragLeave, DragOver)
  • Drop
  • HoldingEvent

I'm not sure what the current state of the world is for drag and drop in React Native, but I know it exists. Investigating that and figuring out how to make it support more platforms seems like a good approach there.

CapturePointer

  • CapturePointer

Do you think this needs to be attached to an existing component? Could there be another component / JS Module for now that you wrap whatever child views you want and it has behavior to capture or release the pointer? Then it could be made more cross-platform / standardized.

Visibility

  • Visibility I've seen requests for css's visibility property support in React Native. Is it the same as that? In React Native the typical answer has been to just render null instead if you don't want the component to be visible. Is that sufficient?

TheSavior avatar Jan 17 '20 19:01 TheSavior

Thanks for clarifying some of the ways we might need to think about approaching these types of platform properties and APIs, @TheSavior.

We've had a discussion or two internally and wanted to share out our thoughts on how we might start approaching platform specific props.

Platform Specific Views vs Components

In your breakdown above, you've called out two good distinctions for platform specific props - that some should exist as:

  • A View prop
  • A stand-alone platform component

We've been thinking a lot more about how to handle the properties that would typically go on a View component, rather than a separate component itself - so we'll have to dig more into how to handle and determine what should be a platform specific components - but here's some of our thinking:

Platform Specific Views

We already do this in React Native for Windows.

There is a separate/extended View component called <ViewWindows/> that we're currently looking at being the solution to where we put our platform specific properties that make sense to exist as a View prop in general.

This means in order to leverage properties that exist only on the Windows platform in React Native, we would follow a pattern similar to what you proposed here, where a developer would platform select the views they need depending on the platform they're refining for:

import {View} from 'react-native';
import {ViewWindows} from 'react-native-windows';

MyComponent(props) {
  const child = (
    <MyChildComponent>
      <Text>Whee!</Text>
    <MyChildComponent />
  );

  if (Platform.OS === 'Windows') {
    return <ViewWindows onDragEnter={this.handleOnDragEnter()}>{child}</ViewWindows>;
  } else {
    return <View>{child}</View>;
  }
}

Platform Specific Components

As mentioned above, this is a less discussed area for now. Since we've been moving quickly, we've also been creating these types of components on an "as needed" basis and aligning with the core when we notice that whatever platform specific property/API we've made is now in core.

For obvious reasons, this isn't really a feasible solution long-term, but understanding what and when platform specific components should become part of the core is the crux of the open question expounded on below.

Open questions

Lifecycle of platform specific properties

Currently our largest open question in this space is what is the lifecycle - either as a platform View prop or as a separate component - of a platform specific API?

With the new work being done to make React Native work for Mac, we're noticing a lot of crossover between the functionality that's available on the Mac and what's available on Windows - this is most likely because they are both "desktop" environments and therefore share a lot of behavior.

With those two platforms in mind (Windows and Mac), it becomes a question of when do we push platform properties back into core?

We don't want to hold back the development progress for properties being written on <ViewWindows/> or <ViewMacOS/> - which means as work is done in those areas there will be questions around "because this functionality exists in both, should it not be in core?". The counter argument to this of course being everything in core should work on all four platforms.

kikisaints avatar Jan 23 '20 22:01 kikisaints

Thanks for the follow up. I think the distinction I had in my head was that instead of having a <ViewWindows /> with windows specific props for view like onDragEnter, I was thinking that maybe those shouldn't exist on View at all.

Maybe there should be a generic cross platform <DragAndDrop /> component that has that API.

Does the drag and drop functionality specifically need to be a part of View (and thus every component in the ecosystem)? Or could you have a separate component that you wrap the pieces where you want to handle drag and drop?

This seems to be how Drag and Drop is handled in the ecosystem today:

  • https://github.com/mohebifar/react-native-easy-dnd
  • https://www.npmjs.com/package/react-native-draggable
  • https://www.npmjs.com/package/react-native-drag-drop-and-swap

TheSavior avatar Jan 23 '20 22:01 TheSavior

<ViewWindows windowsAccProp1=... onBeginDrag=... onMouseEnter=... onKeyDown=.../>

vs

<Acc Prop1=...>
  <Draggable onBeginDrag=...>
    <Hoverable onMouseEnter=...>
      <Keyable onKeyDown=...>
        <View/>
      </Keyable>
    </Hoverable>
  </Draggable>
</Acc>

Some questions: Does the perf cost of having so many extra nodes on the react side start adding up. Can we avoid ending up with the native tree ending up with so many nodes too? [Probably -- this is similar to the view flattening stuff that android uimanger did (does?)]

I think if we went with the separate components, its a requirement that they do view flattening behind the scenes, since on the native side you'll get behavioral differences if the properties are not all on the same element. As an example accessibility tools will read the props of the focused element, so having a wrapper there doesn't help, it needs to be the item with focus that has those props. Similarly if the wrapper was adding a platform specific accessibility property, then that likely needs to be on the same element as the ones specified in the view its wrapping.

acoates-ms avatar Jan 23 '20 23:01 acoates-ms

Cc @sebmarkbage who has thought deeply about this hierarchy and cross platform views in the past.

My understanding is that Seb has been preferring that we do that nested hierarchy instead of all the additional props on View.

What I'm hearing from some people working on Fabric is that it will be possible to flatten those nodes in native once we have Fabric.

On the React side, I'd imagine that since Seb has been wanting these hierarchies, he must not be concerned about the overhead on the React side or has some plan to avoid that.


FWIW, I think there are some cases where it makes sense to add a prop to View today. It is possible that Visibility is one such example. But we don't necessarily need to push everything there, especially when there is a set of related props that could reasonably live on their own.

TheSavior avatar Jan 23 '20 23:01 TheSavior

Thoughts from @sebmarkbage (from an in person conversation), hopefully repeated by me fairly:

  • We should go for the nested approach instead of additional props on View.
  • Re the concern about flattening, the amount of effort required to flatten in user land might be very complex, instead we should just make having wrappers very cheap.
  • Sebastian thinks that if these just render layout only nodes, they may already get flattened by the algorithm in Fabric (we aren't 100% sure on this)
  • Events in particular are very complicated because they often are timing interdependent. Having these all on View can get very complicated to understand where as having the nesting makes it very clear the ordering of how these would fire, how stop propagation might work, etc.
  • Long term we probably want something different for events to better support this interdependence like React Flare, so in the mean time we should take the path of least resistance which is having separate components instead of adding to View.

TheSavior avatar Jan 30 '20 19:01 TheSavior

Since you were talking about it above, I wanted to share my approach to drag-and-drop. I needed it for my app and was not happy with the existing solutions. Over the past month or two I developed a library for it (which is still in early release and experiencing changes).

Repo: https://github.com/nuclearpasta/react-native-drax Concept explanation: https://github.com/nuclearpasta/react-native-drax/blob/master/docs/concept.md

One catch I ran into was that drag-and-drop of necessity implies being able to drag a view anywhere. Simply using translation styles is not enough because when the dragged view is nested, parent views may clip. My library renders a duplicate copy of the view which hovers over everything during the drag.

That said, I took the "add props to a View" approach, but I have no problem with the wrapping in extra nodes for capabilities paradigm.

lafiosca avatar Jan 30 '20 23:01 lafiosca

Hey all! Following up on this based on the discussion we had in the beginning of March with Facebook and co. about this issue.

We (React Native for Windows) recognize that the approach @TheSavior has detailed is extremely likely to be the permanent solution to platform specific property handling in the future; however, at this time there are still a lot of unknowns that have concerns for us around it.

Therefore, to not block on this we are going to proceed ahead with the approach outline in my previous comment with it known that we may be doing throwaway work, because once the new platform specific approach React is working on gets finalized and makes it's way into implementation for React Native core, we will have to align on that.

kikisaints avatar Mar 16 '20 16:03 kikisaints

I want to come back to this conversation with a concrete example to ensure we have agreement on how we want to handle these platform specific props in the short term.

This PR is adding a new prop to Text for Android only to change where hyphens get added.

Based on this thread, I think there are three options we've talked about:

  1. android_hyphenationFrequency - Prefixed with the platform it is supported on
    1. I strongly prefer this option. I want to let the platforms innovate and provide the best experience on that platform and not be slowed down by other platforms. Ideally there would be a clean API that works on all platforms, but I don't want to block progress on a committee of platform maintainers agreeing on an API design up front. We have some prior art for this now as Pressable has android_ props.
  2. hyphenationFrequency - This is what we've done in the past. Unprefixed, you only know which platform it supports by reading documentation
    1. Unprefixed from the beginning feels bad because it hasn't considered any other platforms and forces other platforms into an API choice that likely doesn't make sense.
  3. Create a new AndroidText component and name the prop hyphenationFrequency
    1. This may make sense for out-of-tree platforms like ViewWindows, but the ship has kinda sailed for ios/android. This approach is out of scope for adding this prop, it would have to be a separate consideration to rip apart all of the props. I'm going to ignore this option for now.

Is there any disagreement that we should proceed with #1? Other options or considerations we have to figure out before being able to make a decision?

I think there are some decisions I don't have obvious answers to, but could probably be figured out later (although I'm open to ideas now too 😀)

  • How should the prefix change when a prop is supported on multiple platforms, but still not considering all? For example, something that started on windows, then goes to mac, but still isn't considering mobile.

TheSavior avatar Jun 26 '20 00:06 TheSavior

I think the platform prefix makes sense as long as including it on a different platform (for example, including an android prop in an iOS app) doesn't cause any errors or warnings

for multi-platform props, it seems like you'd have to come up with a standard but it could get really complicated. Something like:

  • windows_
  • mac_
  • desktop_ (for both Mac and Windows)
  • mobile_
  • etc

gwmccull avatar Jun 26 '20 01:06 gwmccull

The problem I have with platform prefix is that the person implementing it on one platform isn't necessarily going to know if the other platforms are going to be able to support the prop in the future. For example, when I worked on React Native Windows and was implementing a feature supported by Android but not iOS I had to use the Android prefix on Windows. Personally I prefer option 2 as it seems more future-proof.

kevinvangelder avatar Jul 02 '20 21:07 kevinvangelder

I get the future-proof-ness of 2, but I think option 1 will push us more to make things supported for all platforms. I think if there is something that is very specific to one platform/formfactor, then the prefix makes sense. If there is something that just isn't built on RN or supported by the platform, if we have no prefix we might "forget" to implement it when it's finally supported. But if we see it always with the prefix, that might motivate people to implement it for other platforms too and get the prefix removed once it's done. So I think option 1 is kinda our best option 🤔.

pvinis avatar Jul 03 '20 21:07 pvinis

Unfortunately, the reality of OSS is that option 1 will result in a massive divergence of properties between platforms and there is a very slim chance of the cleanup happening if/when the other platforms catch up (and in the mean time, they'll all be adding their own prefix which will cause the author to think they can diverge in implementation because it's specific to their platform).

kevinvangelder avatar Nov 24 '20 00:11 kevinvangelder