UIE-120 Replace colors with useThemeFromContext in ErrorView and dependants. #4611
Conversation
src/components/ErrorView.js
Outdated
| @@ -15,7 +14,6 @@ export const styles = { | |||
| }, | |||
| jsonFrame: { | |||
| padding: '0.5rem', | |||
| backgroundColor: colors.light(), | |||
There was a problem hiding this comment.
Since useThemeFromContext is a hook, it needs to be used in a component. Is there any better way of moving backgroundColor out of style and into inline.
There was a problem hiding this comment.
my thought would be to change styles here to be a function that accepts the colors object as an argument and returns the desired style props object..
There was a problem hiding this comment.
Oooh, I like that. Let me see if I can put something together.
There was a problem hiding this comment.
Oh nice, this was my only feedback and it looks like you're already on top of it
src/components/ErrorView.js
Outdated
| @@ -45,7 +46,7 @@ const ErrorView = ({ error }) => { | |||
| return h(Fragment, [ | |||
| json.message && div({ style: { marginBottom: '1rem' } }, [json.message]), | |||
| div({ style: { fontWeight: 600, marginBottom: '0.5rem' } }, ['Full error:']), | |||
| div({ style: styles.jsonFrame }, [JSON.stringify(json, null, 2)]), | |||
| div({ style: { ...styles.jsonFrame, backgroundColor: colors.light() } }, [JSON.stringify(json, null, 2)]), | |||
There was a problem hiding this comment.
Is this backgroundColor a change or just a refactoring of where it comes from?
There was a problem hiding this comment.
and I guess, is there a reason for attaching it directly to the component instead of making it part of a style (or indeed, the jsonFrame style given the updates below too)?
|
|
Jira Ticket: UIE-120
Summary of changes:
What
Why
Testing strategy