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

core: frontend: Change icon and tooltip for SDP URL copy button #1514

Conversation

joaoantoniocardoso
Copy link
Collaborator

image

@Williangalvani
Copy link
Member

I feel like content-copy makes more sense. I'm pretty sure I've seen it used for link plenty of times
https://pictogrammers.com/library/mdi/icon/content-copy/

@joaoantoniocardoso
Copy link
Collaborator Author

I feel like content-copy makes more sense. I'm pretty sure I've seen it used for link plenty of times https://pictogrammers.com/library/mdi/icon/content-copy/

Icon names are not always the best guess for their use, but would the content-copy be better suited than file-link in this case?

@patrickelectric
Copy link
Member

patrickelectric commented Mar 1, 2023

file-link makes more sense to me for this case, where it's a link for the file.

@patrickelectric
Copy link
Member

@joaoantoniocardoso @Williangalvani what is the status of this PR ?

@joaoantoniocardoso
Copy link
Collaborator Author

@patrickelectric I was waiting for @Williangalvani to show examples or further reasoning against the icon change.

Copy link
Collaborator

@ES-Alexander ES-Alexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :-)

Willian, if we were showing the file url here then I'd agree that content-copy would make more sense (since there would be clear content being copied), but since it just says "File URL" with a download button then I think the file-link makes more sense in this case.

Copy link
Member

@Williangalvani Williangalvani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I dont have THAT strong feelings about this

@joaoantoniocardoso joaoantoniocardoso merged commit b4ee237 into bluerobotics:master Mar 2, 2023
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Mar 8, 2023
ES-Alexander added a commit to ES-Alexander/ardusub-zola that referenced this pull request Mar 9, 2023
patrickelectric pushed a commit to bluerobotics/ardusub-zola that referenced this pull request Mar 9, 2023
@ES-Alexander ES-Alexander added docs-complete Change documentation has been completed and removed docs-needed Change needs to be documented labels Mar 9, 2023
patrickelectric pushed a commit to bluerobotics/ardusub-zola that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-complete Change documentation has been completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants