-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Plugins (wpcom): Update design and text #4995
Conversation
Previously, it was noted that there isn't much to distinguish external links on the WordPress.com plugins page. This patch adds the behavior that a plugin Gridicon will change when hovering over the plugin to show the external glyph if the plugin link is outside of Calypso. Currently this only impacts the Google Analytics plugin on the Business plugin panel. I haven only included the code changes, therefore, on the business plugin component. If we include external links in the future on the other classes of plugins, we can add the same changes there.
Thanks Dennis, the icon replacement is working well. |
Previously we had three separate plugin components for the three plan levels: Free, Premium, and Bussiness in order to facilitate differences in the underlying representation of those features. The differences ended up being insignificant and this patch merges them into one generic plugin component to reduce code size and surface area for bugs to appear.
@drw158 I have merged the plugin components and now this PR has more red than green! |
All looks good to me design wise. The only doubt I have is this:
Can I ask why green instead of our medium blue which seems equally "highlighting" but at the same time more aligned with the overall page look? |
First iteration was blue, but then it looked like "too much blue." Second iteration was gray as shown in the "before" screenshot. We received feedback that the active plugins may not look very active, so I changed it to green. I also thought it was a good opportunity to introduce some color other than blue. But, if the green does not look good, I'd be fine with going back to gray. |
@drw158 wonder if this is a chance to try the color-code @adambbecker was exploring for plans (a color for each plan, basically). |
Worth a shot! @adambbecker can I just pick the colors from the screenshot on your mega Plans post a few weeks ago? Edit: Looking at it again, the Free plan would be yellow, and using yellow for the standard plugin icons might look strange — like something was wrong. It's a good idea for the future, but I'm not sure about for this PR. |
Between "too much blue" and "too much green" I'd go for blue given the "not active enough" feedback, which is fair. |
yeah the whole idea of adding color here because it's "too blue" always seemed strange to me. blue is our entire app's color scheme… |
@@ -18,11 +28,29 @@ export const StandardPlugin = React.createClass( { | |||
descriptionLink | |||
} = this.props; | |||
|
|||
const { isUnderMouse } = this.state; | |||
|
|||
const isExternalLink = hasHttpProtocol( descriptionLink ); |
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.
Can we add this as a prop, instead of testing for this?
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.
@gwwar we deliberated on this point and I think it's best to keep the logic contained inside this component. it's only effect is visual and I don't think there's any need to couple that to the "outside world" I like being able to only care about adding the URL in the plugin data structure instead of having to think about how to represent that data.
Blue hover state was intentional, just to emulate a blue text link. I didn't think about doing a lighter green though. I'm not sure where I stand on the color in general. Not sure whether to do green, blue, or gray. |
@mtias, @drw158: Ah, yeah I see what you mean about the plan colors. I think it could work, but I agree with Dave that the yellow might look a little like a warning. I'm also still playing around with trying those colors outside of the plans page & unfortunately the connection just isn't very strong between remembering those colors & associating them with plans (until you see all the plans on the page). Seeing this action, I actually really dig the green because it enforces the active state, but I can also see what others have mentioned that it can be a little much once a lot of plugins are visible. I was playing around a bit here just to see what some other versions could look like and I settled on something like this: Take that with a grain of salt mostly because I was just testing things out. But a few things that caught my eye were:
This is really great though. All of the above is totally up to your discretion, just throwing a few more ideas into the pot just in case you were still exploring. Overall, I do like the green direction. I would also probably remove the blue bar underneath the section header and keep the hover color the same in that particular case. Or, keep the color bar beneath the section header, but make it dynamic so that active sections get green, purchasable sections get blue. |
The green check marks I feel work really really well. We might get away also with the |
👍 to this. The notion of "primary SectionHeader" also didn't make much sense to me. |
The blue bar underneath was to address feedback that the Premium and Business sections did not stand out as "headers", and visually got lost. Currently we don't have a standard way to do something like this. But, if you feel that the header stand out enough, then I'm fine with removing it. |
I also like the green check marks. When I first saw the nudge component in the mockup, I thought that was a really interesting idea. It sort of makes sense in the Calypso context. After letting it sink it though, it does look strange in the context of what a "normal" website looks like. |
# Conflicts: # client/my-sites/plugins-wpcom/plugin-types/business-plugin.jsx
Ok, I changed the active state to blue for now. I will explore the check marks in another PR. I updated from master, so no more conflicts. Are we ok to merge now? |
|
This addresses some feedback. fixes #4986
@dmsnell For 3, can we change the plugin icon on hover, if the link is external? Currently this is all but the Google Analytics plugin. My idea is to change the plugin icon to the
external
one on hover.Another solution would be to add a tooltip for each link.
Before:
After:
Hover state: