iOS: Remove leading space from accessibilityLabel #12269
Closed
Conversation
This file contains 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
facebook-github-bot
added
GH Review: review-needed
CLA Signed
In some cases, the accessibilityLabel contains a leading space. This is because `RCTRecursiveAccessibilityLabel` adds a space before every iteration of the loop including the first.
After this change, the contract is that:
- `RCTRecursiveAccessibilityLabel` always returns a string with a leading space.
- `accessibilityLabel` never returns a string with a leading space.
**Test plan**
I created a test app with the following code:
```
<View style={{height: 100, width: 100, backgroundColor: 'steelblue'}} accessible={true}>
<View style={{height: 20, width: 20, backgroundColor: 'red'}} accessibilityLabel='One' />
<View style={{height: 20, width: 20, backgroundColor: 'yellow'}} accessibilityLabel='Two' />
<View style={{height: 20, width: 20, backgroundColor: 'green'}} accessibilityLabel='Three' />
</View>
```
Before this change, the accessibilityLabel of the outermost View was " One Two Three" (notice the leading space).
After this change, it is "One Two Three" as desired.
rigdern
force-pushed
the
rigdern/iosAccessibilityLabel
branch
from
43761d7 to
3c1c8cc
Compare
|
I believe, it would be better if we will fix it inside |
added 2 commits
February 21, 2017 17:25
This reverts commit 3c1c8cc. Implementing a different solution.
In some cases, the accessibilityLabel contains a leading space. This is because `RCTRecursiveAccessibilityLabel` adds a space before every iteration of the loop including the first. This change fixes it by avoiding adding a space on the first iteration of the loop.
|
@shergin Thanks for the feedback. I pushed a commit which does the fix inside of |
facebook-github-bot
added
the
Import Started
|
@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
facebook-github-bot
added
Import Failed
and removed
Import Started
|
I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification. |
GaborWnuk
pushed a commit
to GaborWnuk/react-native
that referenced
this pull request
Feb 28, 2017
Summary:
In some cases, the accessibilityLabel contains a leading space. This is because `RCTRecursiveAccessibilityLabel` adds a space before every iteration of the loop including the first.
After this change, the contract is that:
- `RCTRecursiveAccessibilityLabel` always returns a string with a leading space.
- `accessibilityLabel` never returns a string with a leading space.
**Test plan**
I created a test app with the following code:
```
<View style={{height: 100, width: 100, backgroundColor: 'steelblue'}} accessible={true}>
<View style={{height: 20, width: 20, backgroundColor: 'red'}} accessibilityLabel='One' />
<View style={{height: 20, width: 20, backgroundColor: 'yellow'}} accessibilityLabel='Two' />
<View style={{height: 20, width: 20, backgroundColor: 'green'}} accessibilityLabel='Three' />
</View>
```
Before this change, the accessibilityLabel of the outermost View was " One Two Three" (notice the leading space).
After this change, it is "One Two Three" as desired.
Adam
Closes facebook#12269
Reviewed By: javache
Differential Revision: D4596761
Pulled By: shergin
fbshipit-source-id: 7d5ff704e858d9f277d1547339a2831ffa90f592
dudeinthemirror
pushed a commit
to dudeinthemirror/react-native
that referenced
this pull request
Mar 1, 2017
Summary:
In some cases, the accessibilityLabel contains a leading space. This is because `RCTRecursiveAccessibilityLabel` adds a space before every iteration of the loop including the first.
After this change, the contract is that:
- `RCTRecursiveAccessibilityLabel` always returns a string with a leading space.
- `accessibilityLabel` never returns a string with a leading space.
**Test plan**
I created a test app with the following code:
```
<View style={{height: 100, width: 100, backgroundColor: 'steelblue'}} accessible={true}>
<View style={{height: 20, width: 20, backgroundColor: 'red'}} accessibilityLabel='One' />
<View style={{height: 20, width: 20, backgroundColor: 'yellow'}} accessibilityLabel='Two' />
<View style={{height: 20, width: 20, backgroundColor: 'green'}} accessibilityLabel='Three' />
</View>
```
Before this change, the accessibilityLabel of the outermost View was " One Two Three" (notice the leading space).
After this change, it is "One Two Three" as desired.
Adam
Closes facebook#12269
Reviewed By: javache
Differential Revision: D4596761
Pulled By: shergin
fbshipit-source-id: 7d5ff704e858d9f277d1547339a2831ffa90f592
dudeinthemirror
pushed a commit
to dudeinthemirror/react-native
that referenced
this pull request
Mar 1, 2017
Summary:
In some cases, the accessibilityLabel contains a leading space. This is because `RCTRecursiveAccessibilityLabel` adds a space before every iteration of the loop including the first.
After this change, the contract is that:
- `RCTRecursiveAccessibilityLabel` always returns a string with a leading space.
- `accessibilityLabel` never returns a string with a leading space.
**Test plan**
I created a test app with the following code:
```
<View style={{height: 100, width: 100, backgroundColor: 'steelblue'}} accessible={true}>
<View style={{height: 20, width: 20, backgroundColor: 'red'}} accessibilityLabel='One' />
<View style={{height: 20, width: 20, backgroundColor: 'yellow'}} accessibilityLabel='Two' />
<View style={{height: 20, width: 20, backgroundColor: 'green'}} accessibilityLabel='Three' />
</View>
```
Before this change, the accessibilityLabel of the outermost View was " One Two Three" (notice the leading space).
After this change, it is "One Two Three" as desired.
Adam
Closes facebook#12269
Reviewed By: javache
Differential Revision: D4596761
Pulled By: shergin
fbshipit-source-id: 7d5ff704e858d9f277d1547339a2831ffa90f592
|
I'm not sure if it's just my setup but I'm still getting whitespaces in my accessibility labels even after applying this patch. Probably due to higher-order components (e.g Redux's Here's a sample output that Could it be that the some components return blank as |
|
@jaunesarmiento What version of React Native are you using? Are you sure it includes the fix? |
|
@rigdern 0.42. I've used the commit as a patch. Yes, the fix is included cause the prior to this fix, the following have leading whitespace in them: |
|
@rigdern nevermind. Found out that it's easier to use |
|
@jaunesarmiento Obviously, you should not use |
|
@jaunesarmiento I know you may not be interested anymore but I suspect PR #14177 will fix your issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some cases, the accessibilityLabel contains a leading space. This is because
RCTRecursiveAccessibilityLabeladds a space before every iteration of the loop including the first.This change fixes it by avoiding adding a space on the first iteration of the loop.
Test plan
I created a test app with the following code:
Before this change, the accessibilityLabel of the outermost View was " One Two Three" (notice the leading space).
After this change, it is "One Two Three" as desired.
Adam Comella
Microsoft Corp.