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

Add icons #7

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add icons #7

wants to merge 2 commits into from

Conversation

kimicol
Copy link
Collaborator

@kimicol kimicol commented Mar 28, 2024

Add icons to actions to improve ergonomy and global aspect.
Related to issue #3

NOT TESTED YET !

Copy link
Owner

@ofaurax ofaurax left a comment

Choose a reason for hiding this comment

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

Can we use icons from a coherent iconset, so that they have the same look'n'feel?

pictures/youtube-svgrepo-com.svg:Zone.Identifier Outdated Show resolved Hide resolved
vertical-align: middle;
float: none;
}

Copy link
Owner

Choose a reason for hiding this comment

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

This can be factorized. Also class names should be semantic: what is icon1? Which use?

@@ -177,6 +198,7 @@
echo '<input type="hidden" name="token" value="'.$token.'" />';
}
?>
<img class="icon1" src="pictures/magnifying-glass-tilted-left-svgrepo-com.svg">
Copy link
Owner

Choose a reason for hiding this comment

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

please use "img" for images, as they are not "pictures" (like photos) but icons

echo ' [<a href="http://www.youtube.com/results?search_query='.urlencode($c.', '.$l[$i+1].', '.$l[$i+2]).'">youtube</a>]';
echo ' [<a href="http://musicainfo.net/quiksrch.php?vol='.urlencode($c).'">musicainfo</a>]';
echo ' [<a href="http://www.youtube.com/results?search_query='.urlencode($c.', '.$l[$i+1].', '.$l[$i+2]).'"><img class="icon2" src="pictures/youtube-svgrepo-com.svg" alt="youtube-logo"> youtube</a>]';
echo ' [<a href="http://musicainfo.net/quiksrch.php?vol='.urlencode($c).'"><img class="icon2" src="pictures/web-svgrepo-com.svg" alt="web-logo">musicainfo</a>]';
Copy link
Owner

Choose a reason for hiding this comment

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

alt attribute should be used to aid visual impaired people.
So you might add a real description, like "youtube logo" rather than an ID.

Can't we use musicainfo logo rather than a generic web one?

It should be checked, but if the image is only used for decoration, I think we can use an empty alt=""

Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't be there

@ofaurax
Copy link
Owner

ofaurax commented Mar 30, 2024

@kimicol
Copy link
Collaborator Author

kimicol commented Mar 30, 2024

https://thenounproject.com/

Request an account to download icon, and need to give credits. To use these icons, a credit section or page must be added to arvin.

Advantage of svgrepo.com is that we don't have to attribute them the origin of icon (see https://www.svgrepo.com/page/licensing/).

@ofaurax
Copy link
Owner

ofaurax commented Apr 9, 2024

OK, so perhaps we can go with icons from svgrepo, but using duotone that we can theme black/pink so that we get something coherent

@ofaurax ofaurax changed the base branch from master to main April 29, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants