Tokens: fix dark mode header

[sohkai]

We were using useTheme() before Main was declared, leading to the theme not being reactive to the appearance set.

In general, I wonder if we should make it a rule that Main is used ahead of any other app logic, to avoid cases like this? For example, both Finance and Voting could fall into this trap if they were to use useTheme() in their App.js.

[bpierre]

We were using useTheme() before Main was declared, leading to the theme not being reactive to the appearance set.

Well found! 🙌

In general, I wonder if we should make it a rule that Main is used ahead of any other app logic, to avoid cases like this? For example, both Finance and Voting could fall into this trap if they were to use useTheme() in their App.js.

Yes we should probably try to move it to index.js or to the App wrapper component, with the other providers. But the problem is that it also requires to be at another level than AragonApi so we can use its hook.

I’m wondering if we shouldn’t throw an error if useTheme is used outside of Theme? It would prevent cases like this one that can be hard to debug.

I’m also thinking that AragonApi could optionally provide a render prop, or perhaps we could only do that for the aragonUI connector, as a dedicated component?

It would allow to declare both the aragonAPI and aragonUI providers in the same component:

import { AragonApiAragonUi, useAragonApi } from '@aragon/api-react'
import { Main } from '@aragon/ui'

function App() {
  const theme = useTheme()
  const api = useAragonApi()
  return (
    // …
  )
}

export default () => (
  <AragonApiAragonUi>
    {connector => (
      <Main connector={connector}>
        <App />
      </Main>
    )}
  </AragonApiAragonUi>
)

Or we could provide a helper function:

import { connectAragonUi, useAragonApi } from '@aragon/api-react'
import { Main } from '@aragon/ui'

function App() {
  const theme = useTheme()
  const api = useAragonApi()
  return (
    // …
  )
}

export default connectAragonUi(connector => (
  <Main connector={connector}>
    <App />
  </Main>
))

[coveralls]

Coverage decreased (-0.1%) to 97.816% when pulling db01663 on tokens-dark-header into d2b9daf on master.

[sohkai]

I’m wondering if we shouldn’t throw an error if useTheme is used outside of Theme? It would prevent cases like this one that can be hard to debug.

Yes! Or at least a warning :).

---

I really like the connector function!