pods-framework / pods

The Pods Framework is a Content Development Framework for WordPress - It lets you create and extend content types that can be used for any project. Add fields of various types we've built in, or add your own with custom inputs, you have total control.
https://pods.io/
GNU General Public License v2.0
1.07k stars 264 forks source link

Suggestion to replace non-React tooltip with pods-tooltip web components #7123

Open heybran opened 1 year ago

heybran commented 1 year ago

Problem to Solve

Make non-React tooltip styles behave consistently. Maybe in future we can replace React tooltip as well to trim down dependencies?

Proposed Solution

As reported on #7118, there's few tooltips (non-React tooltip) that has style/accessibility issue, it would be nice to replace those tooltips with a standalone pods-tooltip custom element so tooltip styles can be encapsulated inside shadowRoot. Another cool thing about web components is that we can safely update styles without worrying it breaking other elements.

I have coded out first version of pods-tooltip, it's looking pretty good so far?

Still few improvements needed, hopefully I can get it done soon.

  1. Best not to overlap with main sidebar
  2. If anchor link inside tooltip, they should be accessed by keyboard
  3. Same as react tooltip, make it visible and close when click outside (a bug at the moment)
  4. Grab an icon that matches the React tooltip one, current icon is just for demo
Screenshot 2023-07-26 at 21 07 24
heybran commented 1 year ago

Notes

TODO

Updates

  1. Replace old png help icon (non-React ones) with a svg icon that was included in pods-tooltip component

    Screenshot of pods-tooltip when focused
  2. Added keyboard support for tooltip itself and any anchor inside, if any

    Screenshot of pods-tooltip when focused

Questions

  1. I enqueue script of pods-tooltip.js on Line935 & 936 https://github.com/pods-framework/pods/blob/fb70bcc5f44a4003e297a1e49541f8bb439854a9/includes/general.php#L935 https://github.com/pods-framework/pods/blob/fb70bcc5f44a4003e297a1e49541f8bb439854a9/includes/general.php#L936 and render that custom element to the DOM on Line 972 https://github.com/pods-framework/pods/blob/fb70bcc5f44a4003e297a1e49541f8bb439854a9/includes/general.php#L972 Since pods_help() is the function responsible for non-React tooltip, and if this solution is accepted, does it mean we can clean up this pods_help(), meaning removing those jQuery related tooltip codes?
heybran commented 1 year ago

Heya @sc0ttkclark

If you see this and have some time, I have a question.

I enqueue script of pods-tooltip.js on Line935 & 936 https://github.com/pods-framework/pods/blob/fb70bcc5f44a4003e297a1e49541f8bb439854a9/includes/general.php#L935 https://github.com/pods-framework/pods/blob/fb70bcc5f44a4003e297a1e49541f8bb439854a9/includes/general.php#L936 and render that custom element to the DOM on Line 972 https://github.com/pods-framework/pods/blob/fb70bcc5f44a4003e297a1e49541f8bb439854a9/includes/general.php#L972 Since pods_help() is the function responsible for non-React tooltip, and if this solution is accepted, does it mean we can clean up this pods_help(), meaning removing those jQuery related tooltip codes?

After this, I get all I need to create a PR for this issue.

sc0ttkclark commented 1 year ago

Yes, we'd need to update pods_help() usage but also what we use in React would be at ui/js/dfv/src/components/help-tooltip.js

Whatever solution we go with here, we should ensure it is accessible and follows the necessary standards for screen readers as well.

Using one tooltip solution for both would be great. I'm not 100% sold on the <pods-tooltip> custom HTML tag and how that affects accessibility.