ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.97k stars 13.52k forks source link

bug: ionicons aren't loaded in react #18847

Closed hntrl closed 4 years ago

hntrl commented 5 years ago

Bug Report

Ionic version: [x] 4.x

Current behavior: Using the <IonIcon /> component will always try to retrieve the svg from the wrong location. As far as I can tell, this is an issue with the new @ionic/react release. Looking in the network tab on chrome, I see two different locations for individual svgs.

md-menu (used internally by ionic): http://localhost:3000/static/media/md-menu.4e10f85b.svg
md-happy: http://localhost:3000/svg/md-happy.svg

Since /svg isn't bundled by CRA, it'll always return the root object.

Expected behavior: <IonIcon /> should pull the svg from the CORRECT location

Steps to reproduce:

  1. Create a new project using create-react-app
  2. Create 2 IonIcon components, one loaded by ionic and one random.
  3. View project by either using npm start or npm run build.
  4. Open the network tab to view sources
class Page extends React.Component {
    render() {
        <IonIcon name='menu' />
        <IonIcon name='happy' />
    }
}
ReactDOM.render(<Page />, document.getElementById('root'))

Ionic info:

Ionic:

   Ionic CLI : 5.2.3 (/usr/local/lib/node_modules/ionic)

Capacitor:

   Capacitor CLI   : 1.1.1
   @capacitor/core : 1.1.1

Utility:

   cordova-res : not installed
   native-run  : not installed

System:

   NodeJS : v10.14.2 (/usr/local/bin/node)
   npm    : 6.10.1
   OS     : macOS Mojave
hntrl commented 5 years ago

The only workaround i've been able to find:

import { happy } from 'ionicons/icons';
...
<IonIcon icon={happy} />
Chbe commented 5 years ago

Hence icon={happy} and not name={happy}. That solved it for me.

Omer-Shahar commented 5 years ago

The only workaround i've been able to find:

import { happy } from 'ionicons/icons';
...
<IonIcon icon={happy} />

However, it doesn't work with "IonBackButton" (when trying to change the default icon). Is there a way to handle that too?

Thanks in advance :)

Omer-Shahar commented 5 years ago

Right now "IonIcon" doesn't work at all. When I add the tag, typescript immediately gives me an error:

Type '{}' is missing the following properties from type 'Pick<[POSSIBLE PROPERTIES]...

I tried adding different properties, but nothing helps.

obedm503 commented 5 years ago

In my case, this started happening when I updated from @ionic/react@0.0.6-3 to @ionic/react@0.0.7

Omer-Shahar commented 5 years ago

In my case, this started happening when I updated from @ionic/react@0.0.6-3 to @ionic/react@0.0.7

Exactly, the same for me. Something is probably broken with the new version...

guiedinger commented 5 years ago

The only workaround i've been able to find:

import { happy } from 'ionicons/icons';
...
<IonIcon icon={happy} />

Did you install the ionicons package?

hntrl commented 5 years ago

@guiedinger Yeah

@Program4Github Using the icon prop the same way in ion-back-button should work. @core/src/components/back-button

sneko commented 5 years ago

We have the same problem on Vue: https://github.com/ionic-team/ionic/issues/18837

Any idea from the core team please?

elylucas commented 5 years ago

Hello,

Starting in React beta 0.0.7 and IonIcons 4.6, the correct way to load and show an icon is by importing the individual icon and setting the icon prop with the imported icon (instead of using the string-based name):

import { star } from 'ionicons/icons';

While I'm not 100% sure for Vue, my hunch is its the same deal if you are on ionicons 4.6 or above.

Let us know if you update to the above method and its still not working.

Thanks

Omer-Shahar commented 5 years ago

@elylucas Thanks for your answer - it does work with IonIcon tags. However, it still doesn't work with IonBackButton... Is there any way to overcome this?

elylucas commented 5 years ago

@Program4GitHub ya, the missing back button (and any other icons used by Ionic components themselves) was fixed in one of the later versions of react beta (7 or 8, don't quite remember). Which version are you on?

Omer-Shahar commented 5 years ago

@elylucas "@ionic/react": "0.0.9", "ionicons": "^4.6.2",

elylucas commented 5 years ago

@Program4GitHub and you are still getting missing back button icons? If so, could you double check in your node modules and make sure those are, in fact, the downloaded versions?

Omer-Shahar commented 5 years ago

@elylucas When I look at the json files inside the node_modules folder I see that the same versions are written. The problem started when I updated "@ionic/react" by running: npm i @ionic/react@latest --save in my project folder...

elylucas commented 5 years ago

@Program4GitHub hmm, strange. Wonder if its some issue from upgrading from a prior beta. If you create a new tabs app using ionic start reacttabs tabs --type=react and check the details page from the 2nd tab, does the icon appear there?

Omer-Shahar commented 5 years ago

@elylucas I just created a new project as you asked, and tried to create an IonBackButton:

import { flash } from "ionicons/icons";
...
<IonBackButton icon={flash} />

However, there is a TypeScript error (and a lint error under the icon parameter) that says: Type '{ ios: string; md: string; }' is not assignable to type 'string'.

elylucas commented 5 years ago

@Program4GitHub Ya, that seems to be a bug, we get the ability to specify a different icon for IonBackButton in the next release.

If you don't specify the icon, do you see the default back arrow?

Omer-Shahar commented 5 years ago

@Program4GitHub Ya, that seems to be a bug, we get the ability to specify a different icon for IonBackButton in the next release.

If you don't specify the icon, do you see the default back arrow?

No. Just nothing...

obedm503 commented 5 years ago

Hey @elylucas, just out of curiosity: what is the reasoning for moving ionicons from dependency to peer dependency along with using the icon prop? Is it the possibility of tree-shaking?

Edit: addressing wrong person

elylucas commented 5 years ago

@obedm503 Yep, tree shaking and only downloading the icon svgs that you actually use.

hntrl commented 5 years ago

@Program4GitHub Is that your issue? If ionicons is a peer dependency that isn't installed surely that's why nothing is showing up for you.

Omer-Shahar commented 5 years ago

@hntrl The IonIcon tag works just fine, so I guess the problem is not related to the ionicons package. The problem is with the IonBackButton tag, which doesn't display anything (not even the default icon).

asalcedo commented 5 years ago

what about ionToast in button property how can i add the icon?

ashleydp commented 4 years ago

@elylucasu I tried the solution above and what's strange is that it works on one page but not on another one that I have. The import statement (on the page that doesn't work) says "All imports in import declaration are unused."

import { americanFootball, basketball, beer, bluetooth, boat, build, flask, football, paperPlane, wifi } from 'ionicons/icons';

Suggestions?

Psvensso commented 4 years ago

Back button (icons) still behaves strangely with this current release (ionic: 4.11.4 / icons: 4.6.3) <IonBackButton icon={{ md: "close", ios: "close" }} defaultHref="/favs" /> Does not work, no icon show up, but removing the icon prop shows the default back arrow icon just fine.

And forcing a Icon in instead of a string does not work either.

import { close } from "ionicons/icons";
...
<IonBackButton icon={{ md: close, ios: close } as any} defaultHref="/favs" />

Edit: Forcing an icon instead of the new settings object actually worked.

import { close } from "ionicons/icons";
...
<IonBackButton icon={close as any} defaultHref="/favs" />
elylucas commented 4 years ago

Hi @Psvensso , you would set the icon to the back button like so

import {close} from 'ionicons/icons';
...
<IonBackButton icon={close} />

And while the md and ios properties are strings, they are not simply the name of the icon, but the path to where the icon is located. These values might change so it's not advised to hard code them.

ashleydp commented 4 years ago

Hey @elylucas the issue is actually with a normal button

import { americanFootball, basketball, beer, bluetooth, boat, build, flask, football, paperPlane, wifi } from "ionicons/icons";

<IonIcon icon="football" slot="start" />

The import statement (on the page that doesn't work) says "All imports in import declaration are unused."

elylucas commented 4 years ago

@ashleydp Hi,

The icon prop takes in the football object imported from ionicons, not the string "football":

And my guess is none of the other icons are being used in the file, so its telling you you don't need to import them.

Psvensso commented 4 years ago

@elylucas Thank you for your quick responses!

It's unfortunate then that the typescript definition requires the {md: string; ios: string; } for the icon prop when it actually works as expected under the hood.

ionitron-bot[bot] commented 4 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.