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

Update toolbar icons #2925

Merged
merged 4 commits into from Apr 25, 2018
Merged

Update toolbar icons #2925

merged 4 commits into from Apr 25, 2018

Conversation

epwinchell
Copy link
Contributor

@epwinchell epwinchell commented Dec 4, 2017

Replacing remaining power state images with font icons; update other font icons to latest Patternfly. (Depends of upgrade to Patternfly Sass 3.23.1)

Before (Host - Power)
screen shot 2017-12-07 at 12 57 43 pm

After (Host - Power)
screen shot 2017-12-07 at 1 30 02 pm

Before (VM - Power)
screen shot 2017-12-07 at 12 57 19 pm

After (VM - Power)
screen shot 2017-12-07 at 1 29 42 pm

Before (Instance - Power)

screen shot 2018-04-09 at 12 06 59 pm

After (Instance - Power)
screen shot 2017-12-07 at 4 46 13 pm

Before (Physical Server - Power)
screen shot 2017-12-07 at 5 29 06 pm

After (Physical Server - Power)
screen shot 2017-12-07 at 4 56 19 pm

Before (Physical Server - Identify)
screen shot 2017-12-07 at 5 29 12 pm

After (Physical Server - Identify)
screen shot 2017-12-07 at 4 56 09 pm

Before (Middleware Server - Power)
screen shot 2017-12-07 at 5 31 53 pm

After (Middleware Server - Power)
screen shot 2017-12-07 at 5 09 04 pm

@miq-bot miq-bot added the wip label Dec 4, 2017
@epwinchell
Copy link
Contributor Author

@miq-bot add_label graphics, enhancement

@serenamarie125
Copy link

Looking forward to seeing this. @epwinchell please tag me once you get the screenshots

@epwinchell
Copy link
Contributor Author

@serenamarie125 Screenshots posted

@epwinchell
Copy link
Contributor Author

@miq-bot add_label ux/review

@epwinchell
Copy link
Contributor Author

epwinchell commented Dec 15, 2017

@serenamarie125 I had to do some guess work because the status/action pairs here: https://www.patternfly.org/styles/icons/ are incomplete. Also, no mention of things like fa-play that were covered in our original google doc.

I think a comprehensive mapping of actions and icons would be really helpful.
Also, "pficon-paused" is surrounded by a circle.. fa-stop isn't.. so should i use "fa-stop-circle-o"? same question for play, etc.

@serenamarie125
Copy link

@epwinchell will look into this with Jenny on Mon/Tue to see if we can help fill the gaps!

@serenamarie125
Copy link

@epwinchell, note that the convention that's being used is that if a status has a corresponding status, we are using the icon inside a circle for the status. Given that:

  • suspend should use fa-pause
  • I have questions on Shelve & what that specifically does, as well as
  • Reset - wondering if using fa-refresh as you are currently doing makes sense. And if so, if we should just use that label?

@epwinchell
Copy link
Contributor Author

@dclarizio Can you chime in?

I have questions on Shelve & what that specifically does, as well as
Reset - wondering if using fa-refresh as you are currently doing makes sense. And if so, if we should just use that label?

@dclarizio
Copy link

@serenamarie125

I have questions on Shelve & what that specifically does, as well as

I have no idea what Shelve is, sorry

Reset - wondering if using fa-refresh as you are currently doing makes sense. And if so, if we should just use that label?

All I know is reset != refresh, so we can't call it that. In addition, the Reset button on most physical computers looks like the Restart image shown above, but without the little clock hands inside (in case we want to take a clue from the real world).

@epwinchell
Copy link
Contributor Author

ping @serenamarie125

@miq-bot
Copy link
Member

miq-bot commented Jan 30, 2018

This pull request is not mergeable. Please rebase and repush.

@serenamarie125
Copy link

@epwinchell so is shelving new functionality? I don't see it in the old screenshots.

We are using fa-camera 📷for snapshot, but shelving seems more complicated than that 🤔

@epwinchell
Copy link
Contributor Author

@serenamarie125 It's not new... but the "before" screenshot is missing. I'll add it tomorrow.

@epwinchell
Copy link
Contributor Author

@serenamarie125 I added the "before" screenshot. Also, I posted some links earlier describing shelving. Thanks.

@epwinchell
Copy link
Contributor Author

@miq-bot add_label gaprindashvili/no

@epwinchell
Copy link
Contributor Author

@miq-bot rm_label unmergeable

@epwinchell epwinchell changed the title [WIP] Update toolbar icons Update toolbar icons Apr 24, 2018
@miq-bot miq-bot removed the wip label Apr 24, 2018
@serenamarie125
Copy link

@miq-bot add_labels ux/approved
@miq-bot remove_labels ux/review

@miq-bot miq-bot removed the ux/review label Apr 24, 2018
Copy link

@serenamarie125 serenamarie125 left a comment

Choose a reason for hiding this comment

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

I think we are good to go for now. I agree with the approach you've taken - use the equivalent icons :)

@epwinchell
Copy link
Contributor Author

@miq-bot assign @himdel

@epwinchell
Copy link
Contributor Author

@miq-bot assign @dclarizio

@miq-bot miq-bot assigned dclarizio and unassigned himdel Apr 25, 2018
@miq-bot
Copy link
Member

miq-bot commented Apr 25, 2018

Checked commits https://github.com/epwinchell/manageiq-ui-classic/compare/538407236a646a4d79753157cbfe01deb9354806~...05cc67d8c35f5d0b99cec0ed1350ff80f514b1f7 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
13 files checked, 0 offenses detected
Everything looks fine. 👍

@dclarizio dclarizio merged commit f646c84 into ManageIQ:master Apr 25, 2018
@dclarizio dclarizio added this to the Sprint 85 Ending May 7, 2018 milestone Apr 25, 2018
@epwinchell epwinchell deleted the toolbar_update branch April 25, 2018 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants