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

Foundation: Support contextual theming and styles #5867

Merged
merged 2 commits into from
Aug 9, 2018
Merged

Foundation: Support contextual theming and styles #5867

merged 2 commits into from
Aug 9, 2018

Conversation

JasonGore
Copy link
Member

@JasonGore JasonGore commented Aug 9, 2018

Pull request checklist

Description of changes

Add support for contextual theming, styles, settings and scoped settings to createComponent. Create examples to demo.

There's a bit of engineering debt accruing here but seeing as we wanted something for Friday and I'm on Shield, we can either leave this open until issues are addressed or merge and follow up.

context

Focus areas to test

Tests to be written (aforementioned engineering debt.)

Microsoft Reviewers: Open in CodeFlow

…ngs to createComponent. Create examples to demo.
@JasonGore JasonGore mentioned this pull request Aug 9, 2018
9 tasks
// TODO: do we want field props? should fields be part of IComponentOptions and used here?
// TODO: should we centrally define DefaultFields? (not exported from styling)
const DefaultFields = ['theme', 'styles'];
return Customizations.getSettings(DefaultFields, displayName, context.customizations);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultfields are default... some components, like Layer, can mark additional fields as customizable (such as "hostId"). How is this piped in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wanted to talk to you about that. I have the TODO on line 106 for exactly that. 😄

>({
displayName: 'CollapsibleSection',
view: CollapsibleSectionView,
styles
Copy link
Member

@dzearing dzearing Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customizableProps: ['foo', 'bar', 'baz']; maybe

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's what I was thinking, assuming we still needed to pipe them in (tied to other comment)

@JasonGore JasonGore merged commit e70852b into microsoft:master Aug 9, 2018
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Foundation: Component Creation
2 participants