Skip to content
This repository has been archived by the owner on Jun 18, 2024. It is now read-only.

Add 'ms-Fabric' base/wrapper component #784

Merged
merged 4 commits into from
Aug 23, 2016
Merged

Add 'ms-Fabric' base/wrapper component #784

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7fb0e3f
Add an 'ms-Fabric' base/wrapper component that sets a base font-famil…
Aug 23, 2016
15627b3
Remove base font-family for now
Aug 23, 2016
1375f47
Merge branch 'master' into miwhea/base-component
Aug 23, 2016
2a8cf34
Merge branch 'master' into miwhea/base-component
Aug 23, 2016
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
14 changes: 14 additions & 0 deletions src/sass/Fabric.Base.Output.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft. All rights reserved. Licensed under the MIT license. See LICENSE in the project root for license information.

//
// Office UI Fabric
// --------------------------------------------------
// Wrapper component to set base typography.

@import "Fabric.Base";

//== Wrapper/base component
//
.ms-Fabric {
@include ms-Fabric;
}
3 changes: 3 additions & 0 deletions src/sass/Fabric.RTL.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@

// Grid
@import './Fabric.Grid.Output';

// Base/wrapper component
@import './Fabric.Base.Output';
3 changes: 3 additions & 0 deletions src/sass/Fabric.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@

// Grid
@import './Fabric.Grid.Output';

// Base/wrapper component
@import './Fabric.Base.Output';
14 changes: 14 additions & 0 deletions src/sass/_Fabric.Base.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft. All rights reserved. Licensed under the MIT license. See LICENSE in the project root for license information.

//
// Office UI Fabric
// --------------------------------------------------
// Wrapper component to set base typography.


//== Wrapper/base component
//
@mixin ms-Fabric {
@include ms-baseFont;
@include ms-fontColor-neutralPrimary;
}
2 changes: 1 addition & 1 deletion src/sass/_Fabric.Typography.Mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// --------------------------------------------------
// Fabric Core Typography mixins

@mixin ms-baseFont() {
@mixin ms-baseFont {
font-family: $ms-font-family-base;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we want to set this just yet. What was the reason for including it here now?

Copy link
Contributor Author

@mikewheaton mikewheaton Aug 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It just seemed reasonable to set the base font-family along with color. We can leave it out and discuss more if you'd like.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Totally agreed. The only reason I'm questioning it now is that some teams consuming Fabric right now use separate definitions/font family names for Segoe than what Fabric currently provides, so this would produce work for them to override. Happy to discuss though--this definitely moves us in the right direction.

Copy link
Contributor Author

@mikewheaton mikewheaton Aug 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'll take it out for now and we can discuss more. I definitely want to set the base font family here, as it will greatly simplify how we do localization and even (finally!) allow for localized fonts in the Fabric JS components.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @mikewheaton. Like I said, I think it's absolutely the right thing to add, and you're absolutely right--this will accrue even greater benefits to other aspects of the project. My concern is that since this will be potentially broadly-impacting, I want to be sure we're careful about which properties we add when and that the appropriate consumers are alerted to the change and can plan accordingly.

-webkit-font-smoothing: antialiased;
}