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

Classifier: Add tooltips to the Button component in ImageToolbar #6636

Merged
merged 3 commits into from
Jan 28, 2025

Conversation

goplayoutside3
Copy link
Contributor

@goplayoutside3 goplayoutside3 commented Jan 20, 2025

Package

lib-classifier

Linked Issue and/or Talk Post

Closes #4483
This is a small survey task enhancement purposely separate from the lib-classifier_survey-task-design-changes branch.

Describe your changes

Add Grommet Tip to the buttons in the ImageToolbar. The styling matches the tips used in lib-user and lib-content.

I aligned the tip to the left of each button because there will always be enough space to the left of the toolbar regardless of screen size.

Here's a video:

tooltips.mov

How to Review

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
  • FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

  • All pages of a FEM project load: Home Page, Classify Page, and About Pages
  • Can submit a classification
  • Can sign-in and sign-out
  • The component is accessible

New Feature

  • The PR creator has listed user actions to use when testing the new feature
  • Unit tests are included for the new feature
  • A storybook story has been created or updated
    • Example of SlideTutorial component with docgen comments, and its story

@goplayoutside3 goplayoutside3 changed the title dd tooltips to the Button component in ImageToolbar Classifier: Add tooltips to the Button component in ImageToolbar Jan 20, 2025
@goplayoutside3 goplayoutside3 added the enhancement New feature or request label Jan 20, 2025
@goplayoutside3
Copy link
Contributor Author

@seanmiller26 please take a look at the video above for design review of the tooltips.

@coveralls
Copy link

coveralls commented Jan 20, 2025

Coverage Status

coverage: 75.596% (+0.007%) from 75.589%
when pulling 12a00e7 on image-toolbar-tips
into 893bdd3 on master.

@mcbouslog mcbouslog self-assigned this Jan 23, 2025
Copy link
Contributor

@mcbouslog mcbouslog left a comment

Choose a reason for hiding this comment

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

@github-actions github-actions bot added the approved This PR is approved for merging label Jan 24, 2025
@mcbouslog mcbouslog removed their assignment Jan 24, 2025
@seanmiller26
Copy link
Contributor

A few minor things:

For styling, can we use Grommet's Caret? The tip box should still expand to the left of the toolbar.
To better match the behavior in PFE, the interaction should have a delay of 200-500ms.

The copy for each label should match the linked issue #4483

Edit tool: Annotate
Pan tool: Pan
Zoom tools: Zoom In, Zoom Out
Rotate tool: Rotate
Reset tool: Reset View
Invert tool: Invert Colors

@goplayoutside3
Copy link
Contributor Author

For styling, can we use Grommet's Caret? The tip box should still expand to the left of the toolbar.

Yes, Grommet does not come with a built-in caret, but I created one from an additional html element (similar to Grommet's Tip Storybook example)

To better match the behavior in PFE, the interaction should have a delay of 200-500ms.

👍 Added an animation delay of 250ms and a short fade-in. I also made sure the tip only appears on devices with hover instead of a touch action. By default, Grommet's Tip appears when you touch a button on a touchscreen, but this doesn't work with the classifier UX. For example, we don't want the tip to persist when someone touches the zoom buttons on a phone.

The copy for each label should match the linked issue #4483

👍 Done

Lastly, I added a tiny amount of space (2px) in between each image toolbar button because they're looking crowded, especially on mobile phones or in the smaller layouts for separate frames, etc. I'll make sure to check-in once this PR is deployed.

@goplayoutside3 goplayoutside3 merged commit d59c4ab into master Jan 28, 2025
7 checks passed
@goplayoutside3 goplayoutside3 deleted the image-toolbar-tips branch January 28, 2025 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved for merging enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEM Subject Toolbar should have tooltip labels on hover
4 participants