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.

Sign up for GitHub

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

Palette not optional #1951

Merged
merged 7 commits into from
Jun 7, 2017
Merged

Palette not optional #1951

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f35e550
Make palette and font required for theme
christiango Jun 7, 2017
e4acb53
Rush change
christiango Jun 7, 2017
0c6b465
Update ThemePage.tsx
dzearing Jun 7, 2017
13d68aa
Rename theme to palette.
christiango Jun 7, 2017
c045d47
Merge branch 'palette-not-optional' of https://github.com/christiango…
christiango Jun 7, 2017
d8d1cf5
Merge branch 'master' into palette-not-optional
christiango Jun 7, 2017
719039d
Merge branch 'palette-not-optional' of https://github.com/christiango…
christiango Jun 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions common/changes/@uifabric/styling/palette-not-optional_2017-06-07-06-56.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/styling",
"comment": "ITheme: Make palette a required prop",
"type": "minor"
}
],
"packageName": "@uifabric/styling",
"email": "[email protected]"
}
11 changes: 11 additions & 0 deletions common/changes/office-ui-fabric-react/palette-not-optional_2017-06-07-06-56.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "ITheme: Make palette a required prop",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
4 changes: 2 additions & 2 deletions packages/office-ui-fabric-react/src/components/Theme/ThemePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { loadTheme, FontClassNames, ITheme } from 'office-ui-fabric-react/lib/Styling';
import { loadTheme, FontClassNames, ITheme, IPalette } from 'office-ui-fabric-react/lib/Styling';
import { Highlight } from '@uifabric/example-app-base';
import { defaultTheme } from './defaultTheme';
import { Callout } from 'office-ui-fabric-react/lib/Callout';
Expand Down Expand Up @@ -115,7 +115,7 @@ export class ThemePage extends React.Component<any, any> {
private _onColorChanged(index: number, newColor: string) {
let { colors } = this.state;
let color = colors[index];
let theme: ITheme = {};
let theme: Partial<IPalette> = {};
Copy link
Member Author

@christiango christiango Jun 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I believe there's a bug here. Either this is a palette (in which case the theme variable should be renames) or we are in correctly creating an object to pass to loadTheme. I assumed palette, so I changed the type, but would like confirmation.

Copy link
Member

@dzearing dzearing Jun 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Huh, you're right. Can you rename this variable to pallete or something? Then you can just loadTheme({ pallete })

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 left it that way to make sure we noticed. I'll update it


color.value = newColor;

Expand Down
8 changes: 4 additions & 4 deletions packages/styling/src/utilities/theme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import {
} from '../styles/index';
import { Customizer } from '@uifabric/utilities/lib/index';
export interface ITheme {
palette?: IPalette;
fonts?: IFontStyles;
palette: IPalette;
fonts: IFontStyles;
semanticColors?: ISemanticColors;
}
import { loadTheme as legacyLoadTheme } from '@microsoft/load-themed-styles';
Expand All @@ -33,7 +33,7 @@ export function getTheme(): ITheme {
/**
* Loads the default global theme definition.
*/
export function loadTheme(theme: ITheme): void {
export function loadTheme(theme: Partial<ITheme>): void {
_theme = createTheme(theme);

// Load the legacy theme from the palette.
Expand All @@ -45,7 +45,7 @@ export function loadTheme(theme: ITheme): void {
/**
* Creates a custom theme definition which can be used with the Customizer.
*/
export function createTheme(theme: ITheme): ITheme {
export function createTheme(theme: Partial<ITheme>): ITheme {
let newPalette = {
..._theme.palette,
...theme.palette
Expand Down