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

web: Use meaningful icons in Storage proposal page #1152

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Apr 16, 2024

During the presentation of #1138, it was seen that the shadow icon chosen for some of the actions opening a dialog created confusion in some attendees when other no-iconized actions opened a dialog too.

Somehow, it was forgotten one of the principles followed in the Agama UI until now: an action (link, button, button dressed up as a link, menu item, etc) should be easily recognizable by users as an element they can interact with in the way they use to do it. I.e., it's not needed to identify if the action opens a dialog, navigates to another page, to another section of the same page, opens a menu, shows or hides a sidebar, etc, since the same action that today opens a dialog in the future might navigate to another page instead.

Proposed changes here fix the issue by replacing these icons with ones that represent the concept of the action or by none when it would hamper more than help. Keep in mind that icons are merely cosmetic aids for sight people, reason why the meaning of the actions must not be determined by them. The UI must still be meaningful if, for whatever reason, icons disappear.

Of course, the ones proposed here can be changed at any point when better suggestions arise. But remember that at this moment we're mainly relying on https://fonts.google.com/icons. We can change our mind in the future, but choosing a consistent library of icons instead of grabbing them from here and there. For example, Lucid icons could be a good candidate for a library replacement, but in a future PLEASE.

Before After
Storage proposal page using shadow icon Storage proposal page using better icons

dgdavid added 3 commits April 16, 2024 12:06
Icons that intend to represent the action's concept instead of how the
UI is going to present the action to the user.
@dgdavid dgdavid requested a review from ancorgs April 16, 2024 11:16
@coveralls
Copy link

coveralls commented Apr 16, 2024

Coverage Status

coverage: 74.817% (-0.002%) from 74.819%
when pulling 17f45a0 on fix-storage-proposal-icons
into 589663f on master.

To somehow keeps the representation that the field is expanded or not.
This should be considered a workaround and must be changed to something
more expressive, meaningful, and usable: a "Basic View | Advanced View"
control.
@dgdavid dgdavid changed the title web: Use meaningful actions in Storage proposal page web: Use meaningful icons in Storage proposal page Apr 16, 2024
Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid merged commit 524aaa8 into master Apr 16, 2024
2 checks passed
@dgdavid dgdavid deleted the fix-storage-proposal-icons branch April 16, 2024 11:33
@ancorgs
Copy link
Contributor

ancorgs commented Apr 16, 2024

I like all the changes

I'm just not sure about the icon for "Find Space". I admit it's a very challenging one, but I believe this one would fit better (absolutely subjective opinion):

tune

Or maybe also any of these other options:

others

I understand none of them is great, but the one on this pull request looks too much like a searching functionality... which "Find Space" is not.

@dgdavid
Copy link
Contributor Author

dgdavid commented Apr 16, 2024

Yeah, it was suspicious to get the PR merged as quick as it was 😉

To be honest, for this case I like none of your proposals. But, I do not like the magnifying glass neither. Moreover, I believe I even do not like the "Find space" label neither, nor the final implementation we have in that dialog.

But, I just wanted to go ahead having in mind that icons should be just an (small, insignificant) aid. Without any intention of play down the importance they deserves, I wouldn't like to focus more than needed now in the tiny details.

That said, let me explain why I have chosen that icon even when I'm not fully happy with it:

  • If you search "scan", you can see that Qr and barcode scan icons are enclosed in the same square

    Screenshot from 2024-04-16 14-13-03

  • I actually wanted to use "Flex Wrap" or "Flex No Wrap", which looks more meaningful to me since I think they could represent empty and full spaces. But, I though adding one of them would raise a discussion 🥲

    Screenshot from 2024-04-16 14-12-29

  • What is more, I liked one icon from Lucid Icons, but I feel I should not invest my time right now in a proposal for switching from one library to another, since I'd like to think slowly about it before make the movement.
    Screenshot from 2024-04-16 14-13-45

But, please, do not hesitate on making a PR if you feel the "Tune" one will be less confusing for the users. You'll have my LGTM for getting rid of the current one 😉

@dgdavid
Copy link
Contributor Author

dgdavid commented Apr 16, 2024

  • I actually wanted to use "Flex Wrap" or "Flex No Wrap", which looks more meaningful to me since they represents empty and full spaces. But, I though adding one of them would raise a discussion

I also was tempted on using the "Qr scan" one, but it's too tied to a widely known concept for using it.

@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
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.

4 participants