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

Icons: the 'amend' icon is incorrect when using the vibrant theme #3613

Closed
claudenbach opened this issue Mar 13, 2020 · 39 comments · Fixed by #3979
Closed

Icons: the 'amend' icon is incorrect when using the vibrant theme #3613

claudenbach opened this issue Mar 13, 2020 · 39 comments · Fixed by #3979
Assignees
Labels
design Needs input from IDS Design Team team: landmark For Landmark issues type: bug 🐛 [2] Velocity rating (Fibonacci)

Comments

@claudenbach
Copy link
Contributor

Describe the bug
The amend icon is different when using the vibrant theme vs the subtle theme. Believe the icon is correct in the subtle theme.

To Reproduce

  1. Using the existing test-form-buttons.html example
  2. Change the definition of a column, to use the icon 'amend'
  3. Click on the more icon to change the theme between Subtle and Vibrant

Expected behavior
Amend icon would be the same regardless of the theme

Version
v4.27.0-dev

@claudenbach
Copy link
Contributor Author

image

@tmcconechy
Copy link
Member

tmcconechy commented Mar 13, 2020

Almost all the icons are different in each theme. That was part of the point of it. So i think that assumption is incorrect.

Is it really more the meaning is way different? I.E. one is like a ledger and one is an edit "pen"? What does amend mean if so so i can describe it to developers.

@tmcconechy tmcconechy added [1] Velocity rating (Fibonacci) team: landmark For Landmark issues type: bug 🐛 labels Mar 13, 2020
@tmcconechy tmcconechy changed the title Datagrid: the 'amend' icon is incorrect when using the vibrant theme Icons: the 'amend' icon is incorrect when using the vibrant theme Mar 13, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.29.x (May 2020) Sprint via automation Mar 17, 2020
@ben-x-dev
Copy link
Contributor

@tmcconechy I'm a bit confused too. It seems the icons' svg html files are built coming from ids-identity, however in the live demo from design.infor.com I'm seeing the icons correctly: https://design.infor.com/code/ids-enterprise/latest/demo/components/icons/example-index?theme=uplift&variant=light

@ben-x-dev
Copy link
Contributor

I also have another icon with this issue, 'update-preview' image that is showing like a refresh icon in uplift
image

@tmcconechy
Copy link
Member

tmcconechy commented Mar 18, 2020

The design.infor.com site shows the uplift style of the icons. And the designers that made these made certain decisions. So i sort of agree that

update-preview -> looks identical to reload
amend -> looks identical to edit and lost its meaning.
locked -> also the locked / unlock icon looks like a hand bag

@inforandy - Could you add this to the design list to check.

@tmcconechy tmcconechy added [2] Velocity rating (Fibonacci) design Needs input from IDS Design Team and removed [1] Velocity rating (Fibonacci) labels Mar 18, 2020
@ben-x-dev
Copy link
Contributor

Hi @tmcconechy any update on this one?

@tmcconechy
Copy link
Member

@inforandy @lucacolumbu - Do we have any design resources available that can correct these icons? Thanks

@inforandy
Copy link

@tmcconechy thanks tim, will have someone take a look

@elizabethhartley
Copy link

@tmcconechy hey tim, these are the correct vibrant icons we intended to replace the subtle icons. when designing the uplift set we got rid of most object-specific shapes, such as the "amend" icon which was specific to a spreadsheet row, and replaced it with the more general action shape, in this case "edit". the same applies to update preview — instead of a context specific shape as seen in the subtle icons, we reduced down to the basic action, which was "update", or "refresh". if these icons do not serve their intended purpose in these contexts, maybe we can point to other icons within the uplift set.

@tmcconechy
Copy link
Member

tmcconechy commented Apr 15, 2020

I see @elizabethhartley i guess the complaint is it had meaning in the other theme that is now not the same. Does that work with that explanation? @claudenbach @nbcp ? I guess my follow up question would be what is the difference between amending and editing? Sounds like a different word for the same action?

As for update-preview? What does that do? Is refreshing the same thing?

@tmcconechy
Copy link
Member

tmcconechy commented Apr 29, 2020

FInal outcome of this issue:

Keep as Is / Change Rejected:
update-preview -> now less object specific so looks similar to reload
amend -> now less object specific so looks similar to edit

Already Fixed:
locked -> also the locked / unlock icon looks like a hand bag

@jamie-norman
Copy link

If I understand this correctly, this seems arbitrary and incorrect. the Landmark implementation of "amend" is for a helper list. The generic "edit" icon doesn't not have the same meaning. This is a regression in my opinion. The icon change at the IDS level did not adequately consider how other products were using it currently and that decision's downstream impact.

@tmcconechy
Copy link
Member

So as i see it the current problems are with:

amend -
update preview -

Can you explain @jamie-norman what those do? It's probably arbitrary to change them to be the same as edit and reload. But i think maybe we could come up with some that capture the usage in a non object specific way (which was the just of the icn changes in uplift). This is my suggestion...

Can we restart with what those two do?

@tmcconechy tmcconechy reopened this Apr 29, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.29.x (May 2020) Sprint via automation Apr 29, 2020
@tmcconechy
Copy link
Member

tmcconechy commented Apr 30, 2020

@jamie-norman could i get an quick explanation of those. We will make a design jira to try and fix it further to better match the case + the design change. https://jira.infor.com/browse/HLPE-363

cc @inforandy

@jamie-norman
Copy link

The "amend" icon was being used for a helper list, which pulls up a select list similar to the way the lookup/select list (search-list icon) works. Not totally sure about "update preview."
Quick gif: https://recordit.co/hFytWvNGTp

@ben-x-dev
Copy link
Contributor

Btw as for update-preview, it was meant to be used as "preview", can we also please change that name to just "preview" to avoid confusion to us all? The meaning is completely off with how it looks just like the refresh icon, why do we even need 3 different icon names for the same icon? refresh, refresh-current, update-preview all looks the same (https://design.infor.com/product/identity/icons).

I believe the preview icon should look similar the way it's done before in https://design.infor.com/code/ids-enterprise/4.26.0/demo/components/icons/example-index?theme=uplift&variant=light since it represents the action better.

see https://jira.lawson.com/browse/LMCLIENT-29584 for more context

@tmcconechy
Copy link
Member

Yeah i see we dont have a preview icon. I would think we would. We maybe have to add a preview icon so there is both. Then add a different amend icon (amend is adding to a ledger as you dont edit it you always add).

@elizabethhartley
Copy link

@tmcconechy I've been using the eye icon for preview in recent designs.

if amending is adding to a ledger, should the icon be add?

we don't want to get back in the habit of being too prescriptive.

@jamie-norman
Copy link

couple of quick ideas for adding to list.

If you want to set up a call, that's fine with me. Should probably include Phillip Patton as well

@aaron-usiskin
Copy link

@jamie-norman those look nice. I will set up a meeting to get me caught up and move this forward. Can you send me the file you created the icons in?

@jamie-norman
Copy link

jamie-norman commented May 4, 2020

helper-list.zip
I created them in Illustrator, dragged em into Sketch, and then exported.

@aaron-usiskin
Copy link

@jamie-norman THANKS

@aaron-usiskin
Copy link

@jamie-norman figma file here √

@ben-x-dev
Copy link
Contributor

Hi everyone, it's been a while is there any update on this one yet? Do we have a draft preview of the new proposed icons? I guess it's enough for me as long as they don't overlap with the meaning of other existing icons to avoid confusion.

@tmcconechy
Copy link
Member

tmcconechy commented May 22, 2020

I found an additional case where the icon has lost meaning. This is in the tree. We used to have a "#icon-tree-node" icon that looked like a base level of a UI tree. Now its just a circle that looks like a radio button that you can select. This is confusing.

Can we change this too?

Compare:
https://master-enterprise.demo.design.infor.com/components/tree/example-select-multiple.html?theme=uplift&variant=light&colors=0066D4

With
https://master-enterprise.demo.design.infor.com/components/tree/example-select-multiple.html

Additional context: #3936 (comment)

@elizabethhartley
Copy link

Screen Shot 2020-05-27 at 10 18 27 AM

here are four options for amend, let me know what you think @aaron-usiskin @jamie-norman

@jamie-norman
Copy link

@elizabethhartley my preference is option 1, since this is used in the context of a list

@elizabethhartley
Copy link

@jamie-norman great! we're merging this into the ids-icons-standard.sketch file in design system/sketch/theme-uplift now

@tmcconechy
Copy link
Member

@elizabethhartley are we fixing any other icons mentioned here?

  • icon-tree-node (lost meaning)
  • update-preview
  • amend (you mentioned)

@elizabethhartley
Copy link

@tmcconechy those are different tickets in our jira board — we'll be dealing with them separately

@tmcconechy
Copy link
Member

I added #3978 to track the two other icon changes getting into EP code.

@tmcconechy tmcconechy moved this from In progress to Pending Review in Enterprise 4.30.x (June 2020) Sprint May 28, 2020
@lucacolumbu
Copy link

A new Amend icon has been designed, and approved:

https://jira.infor.com/browse/HLPE-363

The icon has been merged into the design system here:
infor-design/design-system#447

@tmcconechy tmcconechy moved this from Pending Review to In progress in Enterprise 4.30.x (June 2020) Sprint May 28, 2020
@tmcconechy
Copy link
Member

Added a PR for the changed icon (4.30). Closing this as this is done. Note again i made #3978 for the remaining icons.

PR: https://github.com/infor-design/enterprise/pull/3979/files

@tmcconechy tmcconechy moved this from In progress to Pending Review in Enterprise 4.30.x (June 2020) Sprint May 28, 2020
@tmcconechy tmcconechy moved this from Pending Review to Ready for QA (beta) in Enterprise 4.30.x (June 2020) Sprint May 29, 2020
@jbrcna
Copy link
Contributor

jbrcna commented Jun 10, 2020

@jbrcna jbrcna moved this from Ready for QA (beta) to Done in Enterprise 4.30.x (June 2020) Sprint Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Needs input from IDS Design Team team: landmark For Landmark issues type: bug 🐛 [2] Velocity rating (Fibonacci)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

9 participants