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

BarRight / Left / Center and Context Provider #683

Open
Crash-- opened this issue Apr 24, 2020 · 3 comments
Open

BarRight / Left / Center and Context Provider #683

Crash-- opened this issue Apr 24, 2020 · 3 comments

Comments

@Crash--
Copy link
Contributor

Crash-- commented Apr 24, 2020

In a few applications, at least for Drive, we have to do stuff like this :

import BarContextProvider from 'cozy-ui/transpiled/react/BarContextProvider'
{isMobile ? (
        <BarRight>
          <BarContextProvider client={client} t={t} store={client.store}>
            {MoreMenu}
          </BarContextProvider>
        </BarRight>
)

As mentioned by @ptbrowne here (https://github.com/cozy/cozy-drive/pull/2003/files#r414522850) it could be great if those components handle the context by itself.

Is there any use of the cozy-bar without using the context of the requesting App? Or do we have to pass every time the calling app context?

@y-lohse
Copy link
Contributor

y-lohse commented Apr 27, 2020

I think whenever we use BarLeft/Center/Right, we want the app's context. In some case we don't need it, maybe because we're not using t or anything else that requires the context, but those are rare and would probably benefit from the context as well.

@Crash--
Copy link
Contributor Author

Crash-- commented Apr 27, 2020

We can imagine that if we really don't need the app's context and use the bar's one, we can add a props withAppContext. By default this props is true.

<BarRight>
 {moreMenu}
</BarRight>

//no store / client / t / router injected from the app
<BarRight withAppContext={false}>
{moreMenu}
</BarRight>

@Crash--
Copy link
Contributor Author

Crash-- commented Apr 27, 2020

We should also add MuiCozyTheme to the provided context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants