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.

Sign up for GitHub

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

Jump to bottom

Remove copy tooltip on Text that is not selectable - iOS #13618

Closed
gabrielbull opened this issue Apr 21, 2017 · 4 comments
Closed

Remove copy tooltip on Text that is not selectable - iOS #13618

gabrielbull opened this issue Apr 21, 2017 · 4 comments
Assignees
@shergin
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@gabrielbull
Copy link
Contributor

gabrielbull commented Apr 21, 2017 •
edited

Description

When creating a custom tooltip (could be using react-native-tooltip), if the custom tooltip is within a Text element, the tooltip will inevitably have the Copy item without any way of removing it.

This is because of the lines 239, 240, 241 of react-native/Libraries/Text/RCTText.m, introduced by @shergin on this commit 5d03ff8 .

Reproduction Steps and Sample Code

Create a custom tooltip inside a Text component.

The Text component can have the selectable property set to true or false, it does not affect this issue.

import * as React from 'react';
import { Text } from 'react-native';
import ToolTip from 'react-native-tooltip';

const MyComponent = () => (
  <Text>
    <Text>This is my text.</Text>
    <ToolTip
      actions={[
        {text: 'My', onPress: (): any => null },
        {text: 'Context', onPress: (): any => null },
        {text: 'Menu', onPress: (): any => null },
      ]}
      underlayColor='transparent'
      longPress={true}
      arrowDirection='down'
    >
      <Text>You can long press me for a tooltip.</Text>
    </ToolTip>
    <Text>This is the rest of my text</Text>
  </Text>
);

export default MyComponent;

capture_d ecran_2017-04-21_a_16_31_27

Solution

If the selectable property on the Text component is set to true, the behaviour should remain the same. However, if it set to false, the Copy item should not show in the tooltip.

To fix this, the following lines should be changed to have a condition to only accept the copy item if the text is selectable:

- (BOOL)canPerformAction:(SEL)action withSender:(id)sender
{
  if (action == @selector(copy:)) {
    return YES;
  }

  return [super canPerformAction:action withSender:sender];
}

Additional Information

  • React Native version: 0.43.4
  • Platform: iOS
  • Development Operating System: MacOS
This was referenced Apr 21, 2017
Closed
Closed
@shergin shergin self-assigned this Apr 23, 2017
@shergin
Copy link
Contributor

shergin commented Apr 23, 2017

Thank you, Gabriel!
I will fix that.

@gabrielbull
Copy link
Contributor Author

@shergin If you look here, I have submitted a fix: #13619

@shergin
Copy link
Contributor

shergin commented Apr 23, 2017

@gabrielbull Awesome! Please address the comments. :)

facebook-github-bot pushed a commit that referenced this issue Apr 27, 2017
@gabrielbull @facebook-github-bot
Fixed issue where the copy item would be added to the tooltips on the…
d5a6152 
Summary:
This PR solves issue #13618.

Selectable text still behaves the same way:

```jsx
<Text selectable={true}>yo yo yo</Text>
```

![capture d ecran 2017-04-21 a 17 10 51](https://cloud.githubusercontent.com/assets/671923/25296285/85eba646-26b5-11e7-8773-e5e55ee0d7bb.png)

Text that is not selectable and has a custom tooltip now do not include the copy item:

```jsx
import * as React from 'react';
import { Text } from 'react-native';
import ToolTip from 'react-native-tooltip';

const MyComponent = () => (
  <Text>
    <Text>This is my text.</Text>
    <ToolTip
      actions={[
        {text: 'My', onPress: (): any => null },
        {text: 'Context', onPress: (): any => null },
        {text: 'Menu', onPress: (): any => null },
      ]}
      underlayColor='transparent'
      longPress={true}
      arrowDirection='down'
    >
      <Text>You can long press me for a tooltip.</Text>
    </ToolTip>
    <Text>This is the rest of my text</Text>
  </Text>
);

export default MyComponent;
```

![capture d ecran 2017-04-21 a 17 10 56](https://cloud.githubusercontent.com/assets/671923/25296297/970949ba-26b5-11e7-8378-3bf0289d1a5a.png)
Closes #13619

Reviewed By: shergin

Differential Revision: D4936900

Pulled By: ericvicenti

fbshipit-source-id: 82028b0958c37d63b8a80882196295be4aebecb4
@gabrielbull
Copy link
Contributor Author

Fixed with the PR.

thotegowda pushed a commit to thotegowda/react-native that referenced this issue May 7, 2017
@gabrielbull @thotegowda
Fixed issue where the copy item would be added to the tooltips on the…
414328b 
Summary:
This PR solves issue facebook#13618.

Selectable text still behaves the same way:

```jsx
<Text selectable={true}>yo yo yo</Text>
```

![capture d ecran 2017-04-21 a 17 10 51](https://cloud.githubusercontent.com/assets/671923/25296285/85eba646-26b5-11e7-8773-e5e55ee0d7bb.png)

Text that is not selectable and has a custom tooltip now do not include the copy item:

```jsx
import * as React from 'react';
import { Text } from 'react-native';
import ToolTip from 'react-native-tooltip';

const MyComponent = () => (
  <Text>
    <Text>This is my text.</Text>
    <ToolTip
      actions={[
        {text: 'My', onPress: (): any => null },
        {text: 'Context', onPress: (): any => null },
        {text: 'Menu', onPress: (): any => null },
      ]}
      underlayColor='transparent'
      longPress={true}
      arrowDirection='down'
    >
      <Text>You can long press me for a tooltip.</Text>
    </ToolTip>
    <Text>This is the rest of my text</Text>
  </Text>
);

export default MyComponent;
```

![capture d ecran 2017-04-21 a 17 10 56](https://cloud.githubusercontent.com/assets/671923/25296297/970949ba-26b5-11e7-8378-3bf0289d1a5a.png)
Closes facebook#13619

Reviewed By: shergin

Differential Revision: D4936900

Pulled By: ericvicenti

fbshipit-source-id: 82028b0958c37d63b8a80882196295be4aebecb4
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@shergin shergin

Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@shergin @gabrielbull @react-native-bot