-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
FontAwesome Icons #1342
FontAwesome Icons #1342
Conversation
c4884f4
to
f1dca41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked through the code and found nothing worth nitpicking about. Also pulled it down - I find the site experience slightly too dark now that the colorful icons are gone. Maybe that can be offset by adjusting the background colors and opting for a lot of contrast.
I also found some the icons a bit small-ish. But same here, that can be fixed in a subsequent PR.
I think this is a great step forward. Thank you!
I'm pretty happy to see some progress on the interface. The dark grey icons still don't work well with the old UI color scheme. But this is not new. I guess you know that and the icons are only the first step to a modern looking user interface. On the pages view there is a big monotone "icon carpet". Here it would help to color the status icons to provide a better visual grouping and make it easier to distinguish. Even the primary action (adding a new page) could be highlighted. Some icon symbols are still inconsistent or are hard to understand:
I know that from experience :) Hopefully you now understand my point when we argued about the former UI PR. It feels a bit ironical for me now and I'm kind of frustrated (Just to remind you: I had the same number of commits with a whole new UI, not only the icons). But anyway, I look forward to see an Alchemy look that fits in the present days so that it looks as good as it actually is. This will also help selling it to clients (in terms of deciding for the right CMS, not money). Best |
I just found a further missing relationship. The state "visible/invisible" should visually match the changing action of this state. Another problem I see is that you are using the same symbol for two different things: "visible in navigation" and "show/hide" element. For the navigation thing it maybe makes sense to choose an icon for the object (navigation) and not for the attribute (visibility). BTW: These kind of problems I already ran into and found some solutions for it. |
Does this default element icon provide any information? I think not. That's why I would replace it with a drag-points icon to show draggable affordance (see Screenshots of former PR). |
Thanks, this is all good and valuable feedback. I did not changed colours and sizes of the icons in this PR on purpose. I want to keep the already huge changeset as small as possible. Refining colours and sizes is something we can tackle in a follow-up PR. Please feel free to help with that.
Yes, this was before and this will still be the case. There will never be the perfect icon for lots of stuff. This is the problem with icons in general. Every person has another association with certain icons, especially on a global world. That's why we have tooltips. The icon, the tooltip, the context and the result helps the user to understand what an icon is used for.
They don't provide an icon for re-ordering pages. This was the best icon I could find in their free icon set. If you have something else in mind please make a suggestion https://fontawesome.com/icons?d=gallery&m=free
I choose the compass for the published state because the globe we used before is very ugly (https://fontawesome.com/icons?d=gallery&q=globe&m=free) and compass as an abstraction for "it is navigable" is a good compromise. Currently we use the globe for published state and for languages. Both not perfect, but worked well for us. This is because an icon does not need to be perfect for a certain state or situation. The user will learn by using it.
An action does not need to reflect the resulting status. Publishing something aka. making it public, aka. push into the cloud/the internet, etc. is a good abstraction in my opinion. I think we really need to make compromises for lot of icons.
The icon for "Sites" is the same now. We could use the globe, but this is currently used for languages (which could confuse users if we now use this for something else, and the globe they provide is very ugly ;)
Yes, the dashboard icon is an svg, maybe we can tweak that. I will also look into the sizes. As I said I did not looked into the sizes, because I wanted to keep the changeset small.
I disagree here as well. We do not need to match up the action icon with the resulting status icon. Also the free icon set is limited, so we need to use what they provide us, there is no show/hide element icon beside the eye icon, what I consider a good abstraction. Also, I don't see a problem using the same icon for the page status. It's another context. Also we use this icon now and I want to keep the change to the current icons as small as possible so we don't confuse users too much.
This PR is only about icons, nothing else. This is a good thing. Having one PR that changes too much in the same time is hard to review. Having lots of changes that don't share the same topic in one commit is also not helpful for further maintainability. Doing open source and keeping a maintainable project is hard work and some times things get rejected, not because we do not value you as a person (because we do), but because we have a responsibility for keeping a clean and maintainable project. |
It does provide information. The visibility status of the element. Public elementHidden elementAnd it is also the draggable handle.
I kind of like that idea. They have an ellipsis icon and a th icon (what looks like the icon you had in you PR) My favourite is the ellipsis icon. But, how do we show the visibility state? |
The ellipsis icon will lead to miss interpretation, because it's usually used for menu buttons.
And what's wrong with that? Doesn't it make sense?
This is only because you want to rely on one certain icon font. In terms of design and usability there are better solutions.
This is only half of the truth: Very good:
Good:
Bad, irritating:
That means: If there is no established symbol or metapher that intuitively works, it's better to invent a new ideom instead of missusing the established ones. I can understand you from the development perspective. You want to rely on an open source icon font, because it's easier to implement and to maintain it in the future. |
And that's why I said you will never find a perfect icon: I think the ellipsis is better and I saw this often used for indicating draggability (but, yes also as "more" icon). But there we are again. Icons are comprises. And just relying on the "physical world" won't work anymore. Ask a 20 year old to click the "Diskette" icon to save. That said, Icons only and always work in the context. Take the burger icon as example. People still not know what this is, that's why people started to add the word "Menu" to it. A label that explains the icon. Icons work on it's own only in very rare cases and even then you still have people being confused. Ask Asians to understand the European/American Iconography. They won't understand. That's why we have tooltips for all our icons.
I think it's not always the case, no. The action that causes a result cannot always be represented by the same icon. Example: "Turn a key to lock the door". What icon do you use? The door, the key, the lock, the turning? The door is still closed. How do you show the state of that door? With an "Open Door", an "Unlocked Locked", a "Broken Key"? Speaking of "turning": Have a look at this "Reload" icon. You will find the very same icon is used for "Redo/Undo", "Refresh", "Sync" and "Repeat" (and even more). It's always a matter of the context. Or do you understand the "B" without any context? Within a text editor for sure, but somewhere else? Probably not.
Yes, as a maintainer of an open source project - that needs to support multiple use cases and needs to stay independent from a designer - I need to make this choice. I need to live with that compromise.
Depends on the context. Take example from above. Lots of icons are used for several uses cases. I don't think "Twirled Arrows" are a bad icon for re-organize/re-arrange or sort. Isn't shuffling a playlist also re-ordering? I am very sure the users will understand. And even if not, they will use the mouse to hover that icon and voilá they will know.
Yes, and this is very important to me. Much more then finding the perfect icon. This may be different from you. As a designer you want to find the perfect icon, no matter what. That's fine, that makes you the great designer you are, but as a maintainer of open source software I have more things to consider.
I didn't ignore the fact, I made a well thought compromise. And I am very confident I made a good one. I researched free icon fonts for Alchemy for several years now and I then made a choice. This work is not out of accident. It is the best choice ever? Probably not. Is it a well established, freely usable, known icon font that works very well for us? Yes, it is. But, hey. If you find a free and open source icon font that has all the icons we need, supports open source licensing, has a huge icon set that works for extensions and plugins as well, then please contribute. But using custom icons won't work for us. Alchemy is not a product, it is a free and open source CMS and that's why we need to live with compromise.
Custom made icons is not an option for us. We cannot rely on a designer, that may be, may be not will continue working on Alchemy. Plugin developers may need icons for their plugins, do we want them to hire designers to provide icons for their plugins? Surely not. I am not saying I found the best icons ever. Maybe you are able to find another icon set that fits our needs. Maybe you even find better icons from the FontAwesome icon set. Please, contribute. Thanks for the feedback. I know you care a lot and that's a good thing! |
Wow, what a discussion! So much knowledge and experience on all sides. Thank you @rbjoern84, this is very valuable feedback. However, I feel it would be best if we were to merge this PR so that we have a basis for subsequent work. I see this PR not as the final word - let's change some icons in subsequent PRs, let's change the color scheme in subsequent PRs, let's absolutely increase the icon size in another PR. I understand you feel that your contribution in #1261 was not appropriately valued. That's not true: Without it, @tvdeyen might not have spent the time he has in improving the icon set. Once this is merged, it's a heartfelt invitation for you to help make Alchemy's admin experience better - one small step at a time (because that's how it's done best). ❤️ |
f1dca41
to
bf5eeeb
Compare
@rbjoern84 @mamhoff I made some small changes:
|
bf5eeeb
to
edf3ba6
Compare
To be honest, I'm tired to argue against such a strong personal opinion and pseudo-arguments.
Come on. That justifies all bad icon decisions? That says the icon doesn't matter as long as we have tooltips. This is way to easy. I understand that you want an open source icon font, for reasons I can partially follow. Maybe there is even a way to contribute to it. But please, do not sell and defend your decisions you made caused by limited icon availability as conscious and a good design approach. In some cases it's obviously nothing but a stopgap. When I compare to the icons used in the former PR #1261, the visual outcome is way less intuitive and consistent now. Althought I would have made another decision this is something I can live with. But as long as you are arguing in terms of design in this case you are blocking us from finding a good solution (e.g. contributing to FA to get the needed icons that fit well).
Yes. You're right. The point is, that the current state is not an improvement yet. Maybe of the code, but not the user experience. And just motivating someone else to do the same again and even worse (not the code, I only mean the design) is not very valuable to me. To end up this discussion, your approach of improving it step by step sounds reasonable to me. Go for it. Maybe my mood and time availability will change in the future, but at the moment I have no motivation to contribute. My comprehensive design proposal you do already have. And even the code. Feel free to use it. I would do it the same way anyway. And if @thomas has something else in mind for his baby, I'm out of the game anyways. |
I revisited the icons you proposed and only see some of them being slightly different from mine:
So, I think we are actually not that far apart. 90% of the other icons are the same. Let's merge this and enhance some of the debatable icons afterwards. Ok? /c @mamhoff |
We'll replace the icons with SVG based FontAwesome icons
All FontAwesome style, sizing and transform options are supported.
Instead of the old PNG based icons we use FontAwesome 5 SVG icons.
Instead of using the css class of the hint to change the icon you can pass the FA icon name instead.
Use fontawesome file icon names. Also removes unsupported ones and add new ones.
Returns the fontawesome icon name for given flash message type.
By leveraging the new message_icon_class helper we use the new FontAwesome icon names for flash message icons.
After replacing the PNG based icons with FontAwesome SVG icons we need to adjust the positions for lots of icons. Mostly by removing line-height hacks and fixing absolute positions.
The default text color - that is also used for icons - is too dark/black. By using a slight touch of blue and a 20% opacity it blends better into the UI.
Since we do not use the old PNG based icons anymore, we do not need the CSS rules anymore.
We are not use the icons component for a long time now
We use FontAwesome icon font everywhere now
edf3ba6
to
809b77e
Compare
rebased with latest master |
Yeah, I agree. I do like some of @rbjoern84 s Icons better (the reorder one, as well as the element toolbar). Maybe we can just add the ones we want as SVG icons. Let's merge this for now :) |
Slightly different? Seriously? That would mean the graphical quality and style. The comparisons you found use completely different symbols or metaphors. That's elementary.
I think you are at least 50% far apart. When I even calculate the graphical inconsistency (visual weight, "roundness" and stroke width) and the other views I've still not compared it's even more.
But honestly this discussion I'm really afraid of. I've already spend a lot of time thinking about which icon would fit best and made wise decisions independently from any limitations of a certain icon font, including years of experience in graphic and interface design. You don't need to ask me to merge. If you're convinced, go for it. |
Finally a new vector based icon set 🚀
Builds on top of #1339
Uses FontAwesome 5 vector icons instead of the old bitmap PNG images. Also removes the old icon font we had for some icons already. Removes the jquery-ui icons as well.
Why FontAwesome?
FontAwesome is is well known and well designed free icon set that has been around for a while. The latest 5.0 release added lots of goodies. Instead of using the new and recommended JavaScript - library that replaces all
<i>
tags withsvg
s on page load - I went with the good old icon font that they also provide. This has been the less troublesome approach, as we use Turbolinks and lots of JavaScript for our UI.Screenshots
Notes
This has been a huge amount of work, that's why there are so many commits. Best reviewed commit by commit and visually by pulling this PR and running the dummy app locally.
Superseds #1261