search

zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.3k stars 654 forks source link

Allow copying a username #5166

Closed Aaron1011 closed 2 years ago

Aaron1011 commented 2 years ago

While it's possible to copy a particular message to the clipboard, it's not possible to copy a particular user's username. This would be very helpful when a username contains non-ASCII characters.

hetanthakkar commented 2 years ago

@gnprice Can I work on this feature!?

alya commented 2 years ago

@hetanthakkar1 please pick an issue with a "help wanted" label; this one doesn't have a clear product plan.

alya commented 2 years ago

After a bit of discussion, a good way to solve this issue would be allowing the user to select (and therefore copy) the name in the user profile screen. Currently, it's not possible to select it.

alya commented 2 years ago

@hetanthakkar1 if you're interested in working on this, please feel free to post your proposed approach here.

VishalSharma2000 commented 2 years ago

Hello @alya. If @hetanthakkar1 has not started working on this issue, then I would like to work on this issue.

hetanthakkar commented 2 years ago

Hello @alya. If @hetanthakkar1 has not started working on this issue, then I would like to work on this issue.

Sure go ahed

VishalSharma2000 commented 2 years ago

Thanks, @hetanthakkar1 :smile:

@alya, here is my approach to solving the issue.

Approach

How the Username is being shown.

We have an AccountDetailsScreen component that gets rendered when we click on any username. This component is having AccountDetails as one of the child components, further AccountDetails is rendering the ZulipText component as one of the children. ZulipText is basically the main component that renders the user's full name in the Text component.

Currently, We are passing style and text as props to the ZulipText component where text is the user's fullname.

I have thought of the below ways to solve this issue.

  1. We can pass an onLongPress as one more prop to the ZulipText component which will be pointing to a function and further inside the ZulipText component new props will be passed to the Text Component because we are destructing all the props. In the function, we will set the user's full name to the Clipboard. To notify the user that the Username is being copied to the Clipboard we can show a toast message with the text similar to Username is copied to Clipboard.
  2. We can make the text selectable either bypassing the props from ZulipText, basically configuring only for this case OR we can update the Text core module component inside the ZulipText component so that it will be selectable at all places. I feel the former approach will be better. Further, as per the platform default options, the user will get an option to copy the selected text.

Please let me know if you feel any of the above approaches will be good and then I'll proceed with them. Thank you :smile:

VishalSharma2000 commented 2 years ago

While working on this issue I found the below. If I click on my username from the private message chat then it directs me to AccountDetailsScreen. But I feel the logged-in user should be directed to its Profile Screen right?

Basically, when I click on someone's else username then I should see their account details but If I click on my username then I should see my profile details. But currently, in both cases, I'm been directed to the account details screen, which is also allowing me to send a private messages to myself.

Please let me know if this is been done for a reason or it's an issue.

alya commented 2 years ago

Basically, when I click on someone's else username then I should see their account details but If I click on my username then I should see my profile details. But currently, in both cases, I'm been directed to the account details screen, which is also allowing me to send a private messages to myself.

Thanks for asking! I think for consistency, we should keep the interaction as-is. It's possible for users to want to see their own profile, or send themselves a message.

chrisbobbe commented 2 years ago

@VishalSharma2000, your proposal 2, passing selectable to the

<ZulipText style={[styles.largerText, styles.halfMarginRight]} text={user.full_name} />

in AccountDetails, sounds good! Let's try that. I think it accomplishes the following (from Alya's https://github.com/zulip/zulip-mobile/issues/5166#issuecomment-1064393124) better than your proposal 1:

allowing the user to select […] the name in the user profile screen

VishalSharma2000 commented 2 years ago

Thanks for asking! I think for consistency, we should keep the interaction as-is. It's possible for users to want to see their own profile, or send themselves a message.

Okay, @alya. Thank you for clearing my doubt.

VishalSharma2000 commented 2 years ago

@chrisbobbe I have added the selectable property to the ZulipText component. Here are the screenshots when selecting the text and after clicking on the copy button

  1. Response to the user is taken care of by the system itself by showing the toast message as shown in the 2nd Image.
  2. The text selection gets trigger in two cases i. On Long Press on the text. ii. Double Click on the text.
After Selecting the Username

After selecting text

After Click on copy Option

After clicking copy option

Some points to take care of:

  1. The above images are taken from the android system, since I work on Ubuntu I was not able to test the changes on the ios system.
  2. There is one small issue on the user experience side, after selecting the text either intentionally or accidentally, the user will only be able to unselect the text either by performing any one operation (Translate / Copy / Share in case of Android) or by clicking on the selected text only. Pressing anywhere outside does not unselect the text. On Search, I found this StackOverflow article but there was no correct solution. Other native applications do have the feature to unselect the text by clicking anywhere outside. So, please let me know do we need to take care of this case presently or we can proceed further?

Thank you :smile:

chrisbobbe commented 2 years ago

Great! I'd be interested in a PR with that change.

2. There is one small issue on the user experience side, after selecting the text either intentionally or accidentally, the user will only be able to unselect the text either by performing any one operation (Translate / Copy / Share in case of Android) or by clicking on the selected text only. Pressing anywhere outside does not unselect the text. On Search, I found this StackOverflow article but there was no correct solution. Other native applications do have the feature to unselect the text by clicking anywhere outside. So, please let me know do we need to take care of this case presently or we can proceed further?

I'll check out your PR and play with it myself, but I think this isn't likely to make the experience seem worse than it already is (i.e., currently you can't select the text at all). It also seems like the kind of thing that I expect to be hard to debug. So for now, I think we don't need to worry much about it. 🙂

VishalSharma2000 commented 2 years ago

@chrisbobbe I have created the PR, please have a look.

Since this is my first PR, it took me some time to read the documentation regarding commit message, creating pull request

Please let me know if you feel any changes are required. Thank you :smiley: