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

Create context aggregator something like a ClayManager #2353

Closed
matuzalemsteles opened this issue Aug 29, 2019 · 10 comments · Fixed by #4162
Closed

Create context aggregator something like a ClayManager #2353

matuzalemsteles opened this issue Aug 29, 2019 · 10 comments · Fixed by #4162
Assignees
Labels
3.x comp: clay-components Issues related to Clay components. (e.g ClayCard, ClayLabel...) good first issue

Comments

@matuzalemsteles
Copy link
Member

Going back to our conversations we had in #2250 related to Clay's context aggregator. I think we should start implementing this as the number of contexts within Clay is starting to grow.

The implementation may look similar to this.

<ClayManager
    value={{
        // It may just be an initial empty object that can be passed to DataProvider,
        // it helps to cache here and be reused on different pages...
        storage: {},
        // Some settings to enable the use of ModalProvider...
        modal: '...',
        // We may have some setting to enable or disable tooltip...
        tooltip: '...',
        spritemap: '...',
        linkComponent: Link,
    }}
>
   <MyApp />
</ClayManager>
@matuzalemsteles matuzalemsteles added comp: clay-components Issues related to Clay components. (e.g ClayCard, ClayLabel...) 3.x labels Aug 29, 2019
@bryceosterhaus
Copy link
Member

Are you thinking of including this in @clayui/shared or its own package? Primarily I am thinking of how we want to handle importing the package-level contexts and managing those dependencies.

We might want it to be its own package since we don't want people to have to depend on modal when they use shared.

@matuzalemsteles
Copy link
Member Author

Are you thinking of including this in @clayui/shared or its own package? Primarily I am thinking of how we want to handle importing the package-level contexts and managing those dependencies.

I actually want to expose ClayManager and leave it as its own package (@clayui/manager).

We might want it to be its own package since we don't want people to have to depend on modal when they use shared.

Hmm, I don't know if I understand this well but do you mean that if the user depends on the @clayui/manager package we don't want him to depend on modal?

@bryceosterhaus
Copy link
Member

I was referencing whether we would do import ClayManager from '@clayui/manager' or import {ClayManager} from '@clayui/shared'.

The dependencies of ClayManager component would be

  • @clayui/link
  • @clayui/icon
  • @clayui/modal
    etc..

I think we are on the same page though, if it is its own package, we should be fine with a handful of dependencies.

@matuzalemsteles
Copy link
Member Author

Oh I see, yeah I think we're fine to have your own package.

@matuzalemsteles matuzalemsteles added this to the IFI-940 v3.0.0 milestone Sep 11, 2019
@matuzalemsteles
Copy link
Member Author

I am adding this to our final milestone of v3.0.0, it would be interesting to see this in this release but it does not prevent us from adding it in future releases.

@diegonvs
Copy link
Contributor

Makes sense adding a sizing like property on the provider for forcing sizing style variations of some components? It will be like:

<ClayManager
    value={{
        // It may just be an initial empty object that can be passed to DataProvider,
        // it helps to cache here and be reused on different pages...
        storage: {},
        // Some settings to enable the use of ModalProvider...
        modal: '...',
        // We may have some setting to enable or disable tooltip...
        tooltip: '...',
        spritemap: '...',
        linkComponent: Link,
		sizing: 'small'
    }}
>
   <MyApp />
</ClayManager>

and it will force all small properties to be true on Provider's children.

/cc @victorg1991 @sandrodw3

@bryceosterhaus
Copy link
Member

I don't think design is as clear cut as that. I think they typically use a mix of all sizes in an app, so I don't think it'd make sense to default to a different size than the components already offer. Design could probably help answer this though.

@kresimir-coko
Copy link
Member

Hey @bryceosterhaus @matuzalemsteles this sounds like an interesting issue, are we going to proceed with it, or is it off the table?

@matuzalemsteles
Copy link
Member Author

@kresimir-coko I think this is still worth it, we have some contexts out there. This was downgraded at the time, but we can re-validate the cases again and start implementing this.

@bryceosterhaus
Copy link
Member

Yeah I agree with @matuzalemsteles, I think its still worth it and will probably need in the future, but it's not pressing at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x comp: clay-components Issues related to Clay components. (e.g ClayCard, ClayLabel...) good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants