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.

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

UX Improvement: Updated the tooltip on user uploaded file links to display "Download <Filename>..." #19834

Closed
wants to merge 1 commit into from

Conversation

rakshit512002
Copy link
Collaborator

@rakshit512002 rakshit512002 commented Sep 24, 2021

This PR is for the issue #19740

TODO:
To display "Download filename..." as a tooltip on the links of user uploaded files.

Testing plan:
Visual testing and automatic tests.

GIFs or screenshots:

BEFORE:
Tooltip Before

AFTER:
Tooltip

@rakshit512002
Copy link
Collaborator Author

@alexmv
Here I have generated a PR for issue #19740 opened by you, Can you review it ?

@ligmitz
Copy link
Collaborator

ligmitz commented Sep 25, 2021

Can you amend your commit message to follow the Zulip convention. Refer to the docs for specifications.

@garg3133
Copy link
Member

@rakshit512002 You might also want to mark "Download" for translation.

Take a look at how $t is being used in static/js/notifications.js.

@isakhagg
Copy link
Collaborator

isakhagg commented Sep 27, 2021

This PR #19834 is unecessary. Since this issue is already fixed in PR #19805

@rakshit512002
Copy link
Collaborator Author

@isakhagg Currently I am the assignee of issue #19740 hence, I opened a PR to fix it.

@isakhagg
Copy link
Collaborator

@isakhagg Currently I am the assignee of issue #19740 hence, I opened a PR to fix it.

Yes, but it has already been fixed in #19805 and the comments you got on this PR is already corrected.

@rakshit512002
Copy link
Collaborator Author

@isakhagg Your PR is not merged yet so please let me try

@rakshit512002 rakshit512002 changed the title Updated the tooltip on uploaded file links UX Improvement: Updated the tooltip on user uploaded file links to "Download <Filename>..." Sep 27, 2021
@rakshit512002 rakshit512002 changed the title UX Improvement: Updated the tooltip on user uploaded file links to "Download <Filename>..." UX Improvement: Updated the tooltip on user uploaded file links to display "Download <Filename>..." Sep 27, 2021
@rakshit512002
Copy link
Collaborator Author

@ligmitz I have amended the commit message, can you have a look at it?
Please correct me if I am still wrong as it's my first PR.

@isakhagg
Copy link
Collaborator

isakhagg commented Sep 27, 2021

@rakshit512002 You're welcome to try as everyone else. When picking an issue to work on please make sure that someone is not already assigned, to avoid duplicate PR. When you claim an issue which no one is assigned to use:

@zulipbot claim

You should read more in the documentation:
https://zulip.readthedocs.io/en/latest/overview/contributing.html

My recommendation is for you to close this PR and for you to assign yourself to a new issue.

@isakhagg isakhagg removed their assignment Sep 27, 2021
@rakshit512002
Copy link
Collaborator Author

@isakhagg The one who opened the issue #19740 himself assigned it to me so I think I didn't made any mistake by opening the PR for that issue if you have any problem with that please clarify it with @alexmv and please don't spam my PR's comment section.

@rakshit512002
Copy link
Collaborator Author

@isakhagg Extremely sorry for the inconvenience caused to you, I'll be closing this PR now.

@alexmv
Copy link
Collaborator

alexmv commented Sep 27, 2021

Hey, folks! Let's try to remember everyone here is volunteering, and trying to help out. This isn't a competition! :)

I re-assigned the issue because I'd not seen any updates from @isakhagg that suggested they were working on it still. I'm sorry for moving it out from under you if you were working on it at the time! In general, Zulipbot will also un-assign folks after a week of no activity. If you're going to claim something, it's a good plan to keep the issue updated as you work through things, even if it's just with "I'm stuck on..." or "I'm reading up about..."

And @rakshit512002, I don't think you made a mistake opening this PR. It's reasonable to have started working on this after I assigned it to you.

But having two PRs for a single issue isn't reason to argue with each other over who was there first, or who currently has the issue assigned. Both of you are trying to be helpful in tackling issues on an open-source project, and it's worth remembering that everyone else is just trying to be helpful as well -- and adjust your tone accordingly. It's an opportunity to learn from how someone else approached solving the same issue.

Finally, thank you for taking the time to work on the issue -- both of you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants