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

[Feature] Unescape column titles to allow custom html to be shown #1396

Merged

Conversation

akulmehta
Copy link
Collaborator

⚡ PowerGrid - Pull Request

Welcome and thank you for your interest in contributing to our project!. You must use this template to submit a Pull Request or it will not be accepted.


Motivation

  • Bug fix
  • Enhancement
  • New feature
  • Breaking change

Description

This Pull Request unescapes the column title field to allow html to be input in that. This is particularly useful if you do not want the column title to have a string but rather have an icon. Since the column title is expected to be coded in by the developer, I do not think this would be a security issue (i.e. XSS vulnerability)

For example:
If only text is allowed (i.e. currently escapes the title) you would need to label a column for "Visibility" making the size of the column broad like:
image

However, with the above PR, you could replace the column title with some html (e.g. fontawesome icon) like:
image

Moreover, you could even put icons before your text like:
image

Not sure if this would require an update in the documentation - but happy to provide a PR there if needed.

Related Issue(s): #_____.

Documentation

This PR requires Documentation update?

  • Yes
  • No
  • I have already submitted a Documentation pull request.

@luanfreitasdev luanfreitasdev changed the title Unescape column titles to allow custom html to be shown [Feature] Unescape column titles to allow custom html to be shown Feb 6, 2024
@dansysanalyst
Copy link
Member

I've got to ask:

Column titles are set by the developer and are not exposed to the end user - right?

Do we all agree that it is very unlikely, very low risk, that a developer will inject any malicious code on a column title?

I think the doc should mention that it is unescaped, and it allows passing HTML but requires some attention by the dev side.

@akulmehta
Copy link
Collaborator Author

akulmehta commented Feb 7, 2024

@dansysanalyst I cannot imagine a case where a column would not be set by a developer. A dynamic column based on user input would likely not be useful. So then the question of a developer will inject any malicious code on a column title - well if the developer is malicious they would probably be able to do more harm to the codebase beyond this.

I suppose a warning similar to that in the https://livewire-powergrid.com/table/add-columns.html#closure-examples would be sufficient - but not sure if even that is necessary. As mentioned I am happy to PR that in the docs if needed.

In fact, I had PR'ed this into an earlier version as well and it was merged #517

@luanfreitasdev
Copy link
Collaborator

Hello @akulmehta, can you provide documentation about the e() in Columns?

Thank you

@akulmehta
Copy link
Collaborator Author

@luanfreitasdev docs added

@luanfreitasdev luanfreitasdev merged commit 2a6ebf2 into Power-Components:5.x Feb 12, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants