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

Question: Is "i" tag supported as Custom Icon? #6262

Open
vicabhu opened this issue Mar 30, 2024 · 9 comments · May be fixed by #6303
Open

Question: Is "i" tag supported as Custom Icon? #6262

vicabhu opened this issue Mar 30, 2024 · 9 comments · May be fixed by #6303
Assignees
Labels
Component: Documentation Issue or pull request is related to Documentation
Milestone

Comments

@vicabhu
Copy link

vicabhu commented Mar 30, 2024

Hi there.

I have seen that you can actually define a custom icon with the following syntax:

<Button icon={() => <i className="pi pi-times" />}/>

See the forked example:
https://stackblitz.com/edit/awdxtv-ltawe4?file=src%2FApp.jsx

However, that syntax is actually not listed in the Custom Icons page.

Is this syntax actually supported?

If so:

  • I guess the {...options.iconProps} (props injections) is actually not needed (it was not working with them)?

  • Would it make sense to update the documentation to include that syntax?

Thanks in advance!

@github-actions github-actions bot added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Mar 30, 2024
@melloware
Copy link
Member

Agreed either it needs to be supported that way or the docs need to be updated.

@melloware
Copy link
Member

Can I ask what was your idea behind <Button icon={() => <i className="pi pi-times" />}/> If you need an icon like that just use icon="pi pi-times" so I am curious your use case?

@melloware melloware added question Issue contains a question about the use of PrimeReact components or the products it supports and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Mar 30, 2024
@vicabhu
Copy link
Author

vicabhu commented Mar 30, 2024

Hi!

My use case is to have a way to embed an SVG icon from a string (either markup or data URI). This string can change dynamically in runtime, so a solution in compilation time is not an option.

According to the "Custom Icon" page, you can use the "img" tag to embed an icon, which means you can theoretically use an embedded data URI icon. However, the format will not be applied due to the limitations of the "img" tag. Using an "i" tag would fix that (by changing the "content" CSS property?) through the "style" property.

Also, that page mentions that you can use the SVG element to embed an icon. However, if you have that markup in a string, it is complicated to parse that into an SVG element without using "dangerouslySetInnerHTML," which requires a parent element to wrap the SVG. Having the possibility to use the "i" tag with embedded SVG set in "dangerouslySetInnerHTML" would solve that issue as well.

I hope it is clear what I am saying. I can provide you with some examples if you want; just give me a couple of days to be in front of a computer again. :)

Thanks!

@vicabhu
Copy link
Author

vicabhu commented Apr 2, 2024

Hi @melloware

Finally, I have something to show.

In this forked example you can see different ways in which icons can be rendered.

As I said before, my use case is the following:

  1. To have a way to render icons from a string, that string can change dinamically in runtime.
  2. To make those icons fit the format of each Prime React component to avoid customizations per component.
  3. The approach must work offline.

As you can see in the example, the components (dropdown and button) that have as label "i tag embedded SVG markup" almost fulfill my goal. Both use the same constant iconIEmbeddedSvg which renders icons using the i tag and injecting the SVG string via dangerouslySetInnerHTML.

image
image

The only thing I would miss would be to format the icon a bit (size and position) like the ClassName elements do.
But, as you can see, that is a problem that is also shared by the "embedded SVG" elements who embed the SVG directly (as supported in the Custom Icons page). So, I guess this is a Bug?

image

The only other way I have tried that half works is to embed the data uri of the SVG as src of the img tag. But as you can (I think due to the limitations of the img tag) the corresponding color is not applied, so I would discard this option for now and stay with the i tag.

I also tried to replace the CSS "content" value (where the font is displayed accordingly) with the icon's data uri, however, I could not find a way to do it since content is located inside ::before of each icon's class.

As a conclusion, this is the reason why I believe that rendering icons with the "i" tag should be officially supported and listed in the documentation, as it promotes flexibility when rendering svg icons that come from a string that is not possible to do otherwise. The other options to display custom icons ("Material" and "Font Awesome") I have discarded for now because I don't want to be attached to a library and because I need it to work offline (I think with both you must be online for it to work).

What do you think?

Thanks!

@melloware
Copy link
Member

Sounds good to me! Changing the ticket to enhancement. Feel free to research and provide a PR most of hte work is done in the class utils\IconUtils.js

@melloware melloware added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add and removed question Issue contains a question about the use of PrimeReact components or the products it supports labels Apr 2, 2024
@vicabhu
Copy link
Author

vicabhu commented Apr 2, 2024

Thanks!

Is there a guide to make PRs for PrimeReact? That would be the first time I do that.

What about the documentation? The fact that you changed the ticket to "Enhancement" means that someone will update the documentation? Or must I PR that as well?

@melloware
Copy link
Member

if you fix it submit your PR and i will coach your through fixing it. The docs are in this repo too it is really easy to update the examples in \components\doc\customicons and it will get published on next build!

@vicabhu
Copy link
Author

vicabhu commented Apr 4, 2024

Hi @melloware

Since I am not sure what to fix, for now I just updated the docu, I submitted the PR: #6303

Please let me know If I did it correctly.

@melloware melloware added Component: Documentation Issue or pull request is related to Documentation and removed Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add labels Apr 4, 2024
@melloware melloware added this to the 10.6.3 milestone Apr 4, 2024
@gucal gucal modified the milestones: 10.6.3, 10.6.4 Apr 9, 2024
@vicabhu
Copy link
Author

vicabhu commented Apr 10, 2024

Hi @gucal , I have seen that my change in the documentation was not published in 10.5.3 but it is now set to 10.5.4. Why is that? Is there something I missed to do in the PR workflow (like, close this ticket)?

Thank you.

@nitrogenous nitrogenous modified the milestones: 10.6.4, Future Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Documentation Issue or pull request is related to Documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants