openlab-at-city-tech / webworkqa

WeBWorK integration for WordPress and BuddyPress
GNU General Public License v2.0
4 stars 2 forks source link

Add text to thumbs-up icon #153

Closed boonebgorges closed 5 years ago

boonebgorges commented 5 years ago

Add text to the thumbs-up icon: “This helped me.”

See https://www.dropbox.com/s/enr47xuwh1rsico/Screenshot%202019-03-21%2015.32.04.png?dl=0

@jennaspevack @bree-z Question for you - how does this interact with "like" counts? Does it replace the count? See screenshot: Screenshot_2019-04-18_14-26-00

jennaspevack commented 5 years ago

I thought it replaced the counts, but I can't find the details of the request anywhere. :) There isn't enough room to include both without a little design adjustment. I'm not sure what the purpose of this feature is at this point. Are the numbers being used to drive display anywhere? @drdrew42 can you advise?

boonebgorges commented 5 years ago

Vote counts are used for sorting, in the style of Stack Overflow. So, on the Single problem view, questions with the most votes appear at the top of the list.

jennaspevack commented 5 years ago

Okay. Thanks, Boone. I think I've got it. Let's keep the counts on the Recent Activity/Home and search results pages and use the text on the Problem pages. If the user isn't logged in they will see get the current hover text. Since you can't "thumbs up/like" a problem from the Recent Activity/Home and search results pages, can we remove the link there? It doesn't seem to do anything anyway.

boonebgorges commented 5 years ago

Let's keep the counts on the Recent Activity/Home and search results pages and use the text on the Problem pages. If the user isn't logged in they will see get the current hover text.

Sounds good.

Since you can't "thumbs up/like" a problem from the Recent Activity/Home and search results pages, can we remove the link there? It doesn't seem to do anything anyway.

You mean, leave the count+icon, but don't make it an a, so that there's no pointer or tooltip on hover?

jennaspevack commented 5 years ago

Since it currently doesn't do anything I wasn't sure what the action was for the like icon on the Home and results page. Maybe to simplify things, let's just make them consistent across the pages and add the link with pointer and tooltip-- unless @drdrew42 has some objection.

boonebgorges commented 5 years ago

I'm afraid I'm still a bit confused about what's needed here. There are four cases:

  1. Logged in, List view
  2. Logged in, Single Problem view
  3. Logged out, List view
  4. Logged out, Single Problem view

Current behavior: 1a. Displays count + icon. When you hover, you don't see a tooltip, but the cursor transforms to a pointer. Clicking does nothing. (I think this is the bug that @jennaspevack noted.) 2a. Displays count + icon. When you hover, you get the hover styles (filled-in icon). When you click, it toggles the "like". 3a. Displays count + icon. When you hover over the icon, you see the 'Join / login to like' tooltip 4a. Same as 3a

In https://github.com/livinglab/webwork-for-wordpress/issues/153#issuecomment-488030194, Jenna suggests that we replace the count with 'This helped me' only in case 2. IMO this is confusing, because Single Problem view is the place where like counts actually affect results (sort order - see my comment above).

It sounds like we definitely want the following: 1b. Display count + icon (no change), but remove the 'pointer' hover effect. 3b. Keep current behavior 4b. Keep current behavior

For 2b, a possibility is to add the text and keep the count, perhaps with parentheses? Like this: Screenshot_2019-05-01_13-39-07

@jennaspevack Help :)

jennaspevack commented 5 years ago

Sure thing! :)

Your mockup looks great. If it all fits on the line I think it would be fine, but I'll think a little bit more about this.

boonebgorges commented 5 years ago

Thanks, Jenna! I've made the changes and will leave it to your team to see whether I've captured the spirit :-D

bree-z commented 5 years ago

I think this looks good, thanks!

@jennaspevack do you want to review before I close this? I'm also adding a gif below in case that makes it easier.

LikeQuestion

jennaspevack commented 5 years ago

This seems great. I noted in #151 that the tooltip doesn't pass WAVE contrast. And I forgot until seeing Bree's gif that we have that same stying in the thumbs up tooltip to Join/Login to Like. Is it possible to adjust it here, as well?

background-color: #c4c4c4 and text color: #000.

boonebgorges commented 5 years ago

I've made the change.

bree-z commented 5 years ago

This looks OK when I inspect the element -- I see:

.__react_component_tooltip.type-info { background-color: #c4c4c4; color: #000; }

I'm getting 176 WAVE contrast errors when I test the homepage while not logged in, although it seems to be identifying a red #ff0000 against a grey background #ff0000. Does that make sense?

Screen Shot 2019-06-20 at 4 26 48 PM

Thanks!

boonebgorges commented 5 years ago

Weird - I'm not seeing the contrast errors at all. Even if I did, there are no visible tooltip elements (that I know of?) with this color combo. We don't use the 'error' type of tooltip. Can we call this a WAVE false positive, or do I need to figure out a workaround?

jennaspevack commented 5 years ago

I'm not seeing anything either. Could it be from the Dev Inspector?

bree-z commented 5 years ago

I didn't have the browser inspector open -- that's the WAVE tool's inspector at the bottom.

But, I just checked in Chrome and don't get any of these errors - just Firefox. Could this be some some bizarre Firefox/WAVE false positive?

boonebgorges commented 5 years ago

Ah, I see it on Firefox. Really weird - it doesn't appear to be looking at an actually rendered element, just the contents of a <style> tag. Never seen that kind of thing before. I'd suggest that we ignore.

bree-z commented 5 years ago

Thanks, Boone. That sounds fine to me. @jennaspevack does that seem OK to you? Thanks!

jennaspevack commented 5 years ago

Yes. Sounds like a plan.

jennaspevack commented 5 years ago

Yes. Sounds like a plan.