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.

Sign up for GitHub

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

Jump to bottom

Add product icons for service catalog #2313

Merged
merged 1 commit into from
Oct 20, 2017
Merged

Add product icons for service catalog #2313

merged 1 commit into from
Oct 20, 2017

Conversation

spadgett
Copy link
Member

@spadgett spadgett commented Oct 19, 2017 •
edited
Loading

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 19, 2017
@spadgett
Copy link
Member Author

spadgett commented Oct 19, 2017 •
edited
Loading

screen shot 2017-10-19 at 12 18 10 pm

@spadgett
Copy link
Member Author

cc @serenamarie125 @jennyhaines

@spadgett spadgett changed the title [WIP] Add product icons for service catalog Add product icons for service catalog Oct 19, 2017
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 19, 2017
@spadgett
Copy link
Member Author

/assign @jwforres

@jennyhaines
Copy link

jennyhaines commented Oct 19, 2017 •
edited
Loading

Nice! Looks so much better with those icons, @spadgett ! It'll be even more awesome once those icons in the recently viewed section are sized up to 30px.

@sspeiche
Copy link

Don't forget to notice that the display names have been updated too to be more concise.

Looks nice @spadgett Thanks all!

@spadgett
Copy link
Member Author

@sspeiche It looks so much better with your updates

@jennyhaines
Copy link

@spadgett - Just got word from branding that they are doing the very last tweaks to the icons...and have one more on the way. I will get those updated ones tomorrow around noon. Could those make it in before this code freeze? Also, did you try putting the new icons into the ordering panel? It would be nice to see it in there as well. @serenamarie125

@spadgett
Copy link
Member Author

Could those make it in before this code freeze?

Yeah, we'll make sure they get in.

Also, did you try putting the new icons into the ordering panel? It would be nice to see it in there as well.

screen shot 2017-10-19 at 2 03 56 pm

@jennyhaines
Copy link

@spadgett That looks nice at a larger size! Thanks.

@jennyhaines
Copy link

jennyhaines commented Oct 19, 2017 •
edited
Loading

Hi @spadgett - The one icon I'm a little unsure about is the JBoss A-MQ one with the message bubbles. Do you think you could size up the "Recently Viewed" icons to 30px so I can see if the details are still visible on the icon? (Trying to figure out whether we need to up the contrast for low res monitors.)

@spadgett
Copy link
Member Author

The recently viewed icons were previously 27px by 27px. Here is 30px by 30px on a non-retina display:

openshift web console 2017-10-19 14-35-11

@jennyhaines
Copy link

Thank you @spadgett - this is super helpful!

@jennyhaines
Copy link

One more size tweak and I'll get out of your hair for now @spadgett . (I know this is the definition of nit-picky.) Those icons could really go up to 32px. I think that would be the best size. I just tested with square icons). Thanks!

@spadgett
Copy link
Member Author

32px:

openshift web console 2017-10-19 15-11-16

@serenamarie125
Copy link

I really like them in the catalog and in the ordering flow! agree that as recently viewed services, they may not look the best though.

@spadgett what about the search dropdown?

@jennyhaines
Copy link

bitmoji

@spadgett
Copy link
Member Author

openshift web console 2017-10-19 15-12-36

@spadgett
Copy link
Member Author

Ignore the duplicates :) That is from a local config setting

@serenamarie125
Copy link

Looks great @spadgett !!!

@spadgett
Copy link
Member Author

The 32px size change needs to happen in the origin-web-catalog repo. @sg00dwin can you look at that as a fix for openshift/origin-web-catalog#506 ?

@spadgett spadgett mentioned this pull request Oct 19, 2017
Closed
@jennyhaines
Copy link

@spadgett - I'm curious how the 3scale icon looks in the search dropdown? That icon is very detailed.

@spadgett
Copy link
Member Author

I'm curious how the 3scale icon looks in the search dropdown? That icon is very detailed.

I don't think we have anything in the catalog currently using, but I created a dummy service with the icon:

openshift web console 2017-10-19 15-35-23

openshift web console 2017-10-19 15-36-08

openshift web console 2017-10-19 15-36-23

32px:

openshift web console 2017-10-19 15-37-15

@serenamarie125
Copy link

@spadgett thanks for getting these in so quickly!

@spadgett
Copy link
Member Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2017
@spadgett
Copy link
Member Author

spadgett commented Oct 20, 2017 •
edited
Loading

I will get those updated ones tomorrow around noon.

@jennyhaines Any update on the icons? The cutoff for features is today. We could add what we have and update them as a bug fix later if needed.

@spadgett
Copy link
Member Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2017
@jennyhaines
Copy link

jennyhaines commented Oct 20, 2017 •
edited
Loading

Yes @spadgett ! Just got the updated ones! https://drive.google.com/drive/folders/0Byx88a2G0Sw4SGpXLTk0bmZyMDA?usp=sharing

@spadgett
Copy link
Member Author

/hold

Need to update to latest icons.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2017
@spadgett
Copy link
Member Author

Thanks @jennyhaines !

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 20, 2017
@spadgett
Copy link
Member Author

/hold cancel

Updated with the latest. @jwforres PTAL

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2017
@spadgett
Copy link
Member Author

screen shot 2017-10-20 at 3 46 58 pm

@jwforres
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2017
@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit 6bbe4e9 into openshift:master Oct 20, 2017
@spadgett spadgett deleted the product-icons branch October 20, 2017 22:28
@sspeiche sspeiche mentioned this pull request Oct 23, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwforres jwforres jwforres approved these changes

Assignees

@jwforres jwforres

Labels
lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@spadgett @jennyhaines @sspeiche @serenamarie125 @jwforres @openshift-merge-robot @openshift-ci-robot