Replace :hover CSS pseudo class with useHover hook

[so99ynoodles]

Closes #771

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

I added some tests for useHover for its newly created state isHovered.
Since it was created for visual changes, tests focus on visual changes.

🧢 Your Project:

Components

• Buttons
• BreadCrumbs
• Calendar
• DatePicker
• CheckBox
• Link
• ListBox
• Radio
• TextField
• SideNav
• Table
• Tabs
• Tag
• MenuItem

[devongovett]

This is awesome work! Looks like you've got the right idea. 👍

[so99ynoodles]

@devongovett
Thank you for checking!
I think I finished adding useHover to every component that currently exists, could you review it again?
Please let me know if I am missing something.

[snowystinger]

Can TextFieldBase handle isHovered so that we don't need to get it from useSearchField or apply it in SearchField? I imagine this would help cover TextArea/Textfield

[so99ynoodles]

Yes! I'm gonna fix it to use useHover by itself.
However, If I am right, SearchInput needs to pass isHovered down to TextFieldBase since it's search-icon needs hover styling as well.

[snowystinger]

I think the search icon is just passed to TextFieldBase, which should handle applying the hovered style to any icon it gets?

[so99ynoodles]

@snowystinger
Oh, I see, and it would be great!
But to look at CSS, I think TextField itself doesn't handle its icon's hovered style but SearchField does.
So I'm thinking about adding some CSS for TextField.

/* current */
.spectrum-Search-input {
  /* The value of color is identical for hover/active/focus-ring, but we repeat it here in case one is changed in the future */
  &:hover {
    & ~ .spectrum-Search-icon {
      color: var(--spectrum-search-icon-color-hover);
    }
  }

  &:active {
    & ~ .spectrum-Search-icon {
      color: var(--spectrum-search-icon-color-down);
    }
  }

  &:focus-ring {
    & ~ .spectrum-Search-icon {
      color: var(--spectrum-search-icon-color-key-focus);
    }
  }

  &:disabled {
    & ~ .spectrum-Search-icon {
      color: var(--spectrum-textfield-text-color-disabled);
    }
  }
}

/* case 1 */

.spectrum-Textfield-input {
  &:hover {
    & ~ .spectrum-Textfield-icon {
      color: var(--spectrum-textfield-placeholder-text-color-hover);
    }
  }
}

/* case 2 */

.spectrum-Textfield-icon {
  color: var(--spectrum-textfield-icon-color);

  &.disabled {
    color: var(--spectrum-textfield-text-color-disabled);
  }

  &:hover {
    color: var(--spectrum-textfield-placeholder-text-color-hover);
  }
}

I prefer case 1 to case 2.

[snowystinger]

O thanks for pointing that out. It seems like it's a mistake we have not to put the icon styles completely into the basetextfield given that it'd affect textarea/textfield/searchfield. We can address that later, should allow us to get rid of the spectrum-Search-icon class. Don't worry about it here. Thanks for looking into it!
#790

[devongovett]

Awesome work!

[so99ynoodles]

#775 (comment)
I end up merging hoverProps with optionProps in ListBoxOption, if shouldFocusOnHover is false.

// ListBoxOption.tsx
...
      <div
        {...mergeProps(optionProps, shouldFocusOnHover ? {} : hoverProps)}
        ref={ref}
        className={classNames(
          styles,
          'spectrum-Menu-item',
          {
            'is-disabled': isDisabled,
            'is-selected': isSelected,
            'is-selectable': state.selectionManager.selectionMode !== 'none',
            'is-hovered': isHovered
          }
        )}>
...

[snowystinger]

Tests look great, thanks for adding those

[snowystinger]

I think the search icon is just passed to TextFieldBase, which should handle applying the hovered style to any icon it gets?

[snowystinger]

Looks great, I'll wait for @devongovett to look at it again as well since he was also commenting on it

[devongovett]

Thanks @so99ynoodles! This is great work. I adjusted two things:

1. Commented out the documentation changes so they don't deploy until we do the release to npm. We'll handle uncommenting in prep for the release shortly.
2. Disabled the postcss plugin in the docs. We have some static HTML that uses the CSS with no client side JS to apply the .is-hovered class, so there was no hover effect being applied anymore. In production, we build the website based on the published code, which will include pre-compiled CSS, so once deployed it should work as expected. Seems like an ok tradeoff until we can figure out another way to handle this.