lhz516 / react-h5-audio-player

React audio player component with UI. It provides time indicator on both desktop and mobile devices.
https://codepen.io/lhz516/pen/dyGpmgP
MIT License
587 stars 91 forks source link

Consider embedding icons #223

Open ptmkenny opened 5 months ago

ptmkenny commented 5 months ago

At present, the icons in this audio player are loaded via a network request after the player has loaded. This can cause the player to fail in environments where access to the general Internet is restricted (for example, test servers). It also introduces network dependencies on third-party servers, which developers may want to avoid.

It seems this is because of the philosophy of the Iconify for React library:

Loads icons on demand. No need to bundle icons, component will automatically load icon data for icons that you use from Iconify API.

However, react-h5-audio-play only specifies 10 icons, so I think it would be more reliable to bundle all 10 icons with the player.

lhz516 commented 5 months ago

We need internet access to load the web page and play audio anyway. In the testing environment, I don't think it's necessary to show check if icons are displayed.

If you want the icon to be embedded, you consider modifying the customIcons prop and use your own icons.

ptmkenny commented 5 months ago

We need internet access to load the web page and play audio anyway.

This is usually the case, but it's possible to use something like Ionic React to make a mobile app that loads local audio files without Internet access, and in such a case, the audio player will appear broken because no icons are displayed.

I confirmed that the icons will be bundled if you download them and specify them using customIcons, so that's a good workaround.

Perhaps this should be mentioned in the README? Something under "Advanced usage" like:

This component loads its icons via the Internet. If you wish to bundle the icons in your app, you can use the customIcons prop.

lhz516 commented 5 months ago

Sounds good. I can update when I get a chance