-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat(webapp): display fio address instead of owner value #1421
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
- Updated link text in
CountryMap.js
to displayfio_address
if available. - Truncated producer names in
ProducersChart/index.js
to 12 characters. - Updated GraphQL queries in
producer.gql.js
to includefio_address
. - Modified
useEndpointsState.js
to prioritizefio_address
. - Updated
ProfileCard.js
to displayfio_address
if media name is available.
Review
Overall, the changes introduce the usage of fio_address
where applicable, enhancing display information for producers. However, there are potential security issues with XSS attacks and display issues with multi-byte character truncation.
@@ -105,7 +105,7 @@ const ClusterMap = ({ data, map, mapCode }) => { | |||
const producer = this?.point?.producer | |||
|
|||
return producer?.owner | |||
? `<a href=/${eosConfig.producersRoute}/${producer?.owner}>${producer?.owner}</a>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is fine but it would be better to escape the fio_address
and owner
to prevent potential XSS attacks. Consider using a library like DOMPurify.
@@ -105,7 +105,7 @@ const CustomBarLabel = memo( | |||
fontFamily="Roboto, Helvetica, Arial, sans-serif;" | |||
fontWeight={isProducing ? 'bold' : 'normal'} | |||
> | |||
{payload.owner} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The substring operation truncates names but doesn't account for the possibility of cutting in the middle of a multi-byte character. This could cause display issues. Consider using a proper truncation library that handles multi-byte and multi-codepoint characters.
@@ -119,7 +119,7 @@ const ProfileCard = ({ producer }) => { | |||
<div className={classes.profile}> | |||
<ProducerName | |||
name={producer?.media?.name || producer?.owner} | |||
account={producer?.media?.name ? producer?.owner : ''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is fine but it would be better to escape the fio_address
and owner
to prevent potential XSS attacks. Consider using a library like DOMPurify.
Use the fio_address value instead of the owner
Steps to test
Screenshot