Make use of base64 for contributionStatement img #8107
Conversation
| @@ -160,10 +174,19 @@ class ContributionStatement extends ImmutableComponent { | |||
| } | |||
|
|
|||
| get ContributionStatementHeader () { | |||
| const imgStyle = StyleSheet.create({ | |||
There was a problem hiding this comment.
curious: why separate the brave logo style here rather than keeping it with the other styles as before (styles.braveLogo)?
There was a problem hiding this comment.
+1. If we will do so, why won't we do that for all files in a lump to avoid inconsistency? @cezaraugusto would you mind opening an issue to track that?
There was a problem hiding this comment.
method to transform image to base64 is a thenable, so I had to pass result to a state, and styles is not bound to the component, so I can't call this.state there. That's why I had to create a separate stylesheet for each image
| @@ -285,6 +308,17 @@ class ContributionStatement extends ImmutableComponent { | |||
| } | |||
|
|
|||
| ContributionStatementDetailTable (page, pageIdx, totalPages) { | |||
| const imgStyle = StyleSheet.create({ | |||
There was a problem hiding this comment.
same question as above: why not keep this style with the others as before (styles.verified) rather than creating it here?
There was a problem hiding this comment.
pls see comment above
| const braveLogo = require('../../app/extensions/brave/img/braveAbout.png') | ||
| const verifiedIcon = require('../../app/extensions/brave/img/ledger/verified_green_icon.svg') | ||
|
|
||
| class ContributionStatement extends React.Component { |
There was a problem hiding this comment.
why switch from ImmutableComponent to React.Component?
There was a problem hiding this comment.
@willy-b looks like this is now using React's setState(). Our ImmutableComponent wrapper will only trigger a re-render if properties are updated (not state). Changing this forces the setState() to re-render
There was a problem hiding this comment.
Overall, we want to avoid React.Component in favor of ImmutableComponent... but About pages are a little different. We tend to need to keep state, because of how data gets transferred to it (see Frame.js)
There was a problem hiding this comment.
I can't manipulate component-level state without using actions/stores with Immutable. Creating an action/store for that is overkill. Since we're on an about:page there's no big deal to use React.Component
Fix #8062
Auditors: @luixxiul, @willy-b
Note: We still can't make use of external files to append images on contributionStatement if we want to save it.
After component refactor to Aphrodite (#7887) LESS file with hard-coded image string was removed and I missed that case while merging by removing hard-coded string from refactor.
This PR makes use of
getBase64FromImageUrlmethod to solve the issue and avoid hardcoding base64 strings, which could have made style edits harder.You can follow convo here: #7887 (comment)
Test plan: