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

Jmafoster1/237 consistent footer behaviour #698

Open
wants to merge 3 commits into
base: beta-master
Choose a base branch
from

Conversation

jmafoster1
Copy link
Collaborator

Closes https://github.com/GateNLP/we-verify-app-assistant/issues/237.

  • All assistant text result footer icons now have a tooltip on hover (without changing the source of TextCopy and Translate).
  • ExpandMinimise now has pointer cursor to match the other two. It doesn't seem to be used elsewhere so this shouldn't break anything else.

None of the icons seems to visibly "highlight" on hover for me before or after the change.

@jmafoster1 jmafoster1 changed the base branch from master to beta-master January 8, 2025 13:34
Copy link
Collaborator

@rosannamilner rosannamilner left a comment

Choose a reason for hiding this comment

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

Interesting. I can see the hover text on all three, however I can visibly see the highlight on only the TextCopy and Translate. ExpandMinimise does have a Tooltip which should create the highlight?

@jmafoster1
Copy link
Collaborator Author

That's very strange. Perhaps my monitor is just old, but I only see the tooltip text when I hover. I added a tooltip to ExpandMinimise, so the behaviour should be consistent now.
image
image

@jmafoster1 jmafoster1 marked this pull request as ready for review January 13, 2025 09:03
@rosannamilner
Copy link
Collaborator

On your image above I can see the highlight so that's working, and same for TextCopy.
image (92)

It's clearer on TextCopy (perhaps because the icon is smaller).
image (90)

But it still isn't there on ExpandMinimise even with the Tooltip which is strange.

@jmafoster1
Copy link
Collaborator Author

I still could not see it on your image, but I can if I bring the contrast right up! Maybe I need to adjust the settings on my monitor, or my eyes. I'll see if I can work out where the highlighting code is and get that sorted.

@rosannamilner
Copy link
Collaborator

rosannamilner commented Jan 13, 2025

I was wondering if it was my eyes not seeing it for ExpandMinimise haha - maybe if it's hard for us, then perhaps the consistency isn't too important if it turns out to be challenging.

@jmafoster1
Copy link
Collaborator Author

jmafoster1 commented Jan 15, 2025

@rosannamilner, looking at this on my office machine, I can just about see it. I ended up removing the hover from TextCopy and Translate since the icons overflow the highlighted area, making it look very strange to me. The cause of this was that those two tools are Button elements, which change colour by default on hover. ExpandMinimise is just an icon, so has no such behaviour.

EDIT: I re-requested a review from @rosannamilner since I can still only barely see it!

@rosannamilner
Copy link
Collaborator

Now I can't see any highlights on hover for all three - but that is consistent! I just checked the youtube comments on the Video Analysis tool which uses TextCopy and Translate - neither of these have the grey circle hover so perhaps it's best to leave it as they both don't have it.

They are all consistent now with the hover text appearing.

…ugin into jmafoster1/237-consistent-footer-behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants