[0.62] add accessibilityState prop and constructor for related object by sgny · Pull Request #657 · rescript-react-native/rescript-react-native
Conversation
This PR adds the replacement prop for accessibilityStates removed in 0.62.
Before merging, I'd appreciate some feedback on the API. I have chosen to add a module called Accessibility and a type called state with a constructor of the same name. I plan to add another type called value and again a constructor of that name to handle the accessibilityValue prop which also seems to be missing from our bindings.
The JS API necessitates some rather unpleasant choices for the checked field, which can take either a bool or the string "mixed". Since unwrap is still unavailable, this PR defines a type checked in Accessibility, a function to type cast bool to checked and uses bs.inline to handle "mixed".
MoOx previously approved these changes Mar 27, 2020
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in practice this will give
<View accessibilityState=(Accessibility.(state(~disabled=true, ~checked=setChecked(true)))) /> <View accessibilityState=(Accessibility.(state(~disabled=true, ~checked=mixed))) /> <View accessibilityState=(Accessibility.(state(~disabled=true, ~checked=setChecked(false)))) />
That's zero cost but not very fancy.
I don't see another zero cost option.
We could offers something like
<View accessibilityState=(Accessibility.(state(~disabled=true, ~checked=checked))) /> <View accessibilityState=(Accessibility.(state(~disabled=true, ~checked=mixed))) /> <View accessibilityState=(Accessibility.(state(~disabled=true, ~checked=unchecked))) />
But I don't think we can make this zero cost, even if checked & unchecked are functions...
So I guess documentation should be enough for people to find the way to handle this. Maybe we could start with an Accessibility guide with some examples?
I agree, zero cost does not seem possible with a significantly nicer API, at least for now while @bs.unwrap is unavailable for use with @bs.obj. @bs.unboxed does not seem to improve on our "%identity" tricks in this regard.
With that said, we could also define
type checked = bool; let mixed: checked = "mixed"->Obj.magic;
as we have already used elsewhere in this repo. This is again quite low cost.
Then, instead of inlining as string, we could use inlining as bool, as below:
[@bs.inline] let checked = true; [@bs.inline] let unchecked = false;
An appropriate interface would be needed, but perhaps this version would be nicer?
Would let mixed: bool = "mixed"->Obj.magic; be possible (so no checked/unchecked + interface) required?
@MoOx It's certainly possible, but then mixed can be passed to all fields.
Would you want to pass mixed as a boolean everywhere? But yeah this could lead to a runtime error. I am ok for your last proposal then.
MoOx approved these changes Mar 27, 2020
MoOx
deleted the
accessibilityState
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters