Skip to content

Theme generator: show in the website main nav#4251

Merged
lynamemi merged 11 commits into
microsoft:masterfrom
lynamemi:show-theme-generator
Mar 13, 2018
Merged

Theme generator: show in the website main nav#4251
lynamemi merged 11 commits into
microsoft:masterfrom
lynamemi:show-theme-generator

Conversation

@lynamemi

Copy link
Copy Markdown
Collaborator

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

  • The link will appear in the left nav under Styles
  • Blurbs on the Colors and Resources pages that refer/link to the theme generator
  • Added links, header tags, and styles to the theme gen page to be more consistent with the rest of the website
  • Add this to the github url to test without whitespace diffs:

?w=1

Focus areas to test

(optional)

@lynamemi lynamemi requested review from Jahnp and phkuo March 13, 2018 16:54
<div className={ css(pageStyles.u_maxTextWidth, styles.overview) }>
<h2 id='Overview'>Overview</h2>
<p>Fabric includes 9 theme colors and 11 neutral colors. Each has helper classes for text, background, border, and hover states. When selecting colors, refer to the <a className={ styles.colorsPageLink } href={ 'https://static2.sharepointonline.com/files/fabric/fabric-website/files/coloraccessibility_29sep2016.pdf' }>color accessibility guidance (PDF)</a> to ensure that your text can be ready by everyone.</p>
<p>Fabric includes 9 theme colors and 11 neutral colors. Each has helper classes for text, background, border, and hover states. When selecting colors, refer to the <a className={ styles.colorsPageLink } href={ 'https://static2.sharepointonline.com/files/fabric/fabric-website/files/coloraccessibility_29sep2016.pdf' }>color accessibility guidance (PDF)</a> to ensure that your text can be ready by everyone. If you need to customize your theme, see the <a className={ styles.colorsPageLink } href={ '#/styles/themegenerator' }>Theme generator</a>.</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this true anymore? I thought we removed all the helper classes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You tell me :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this blurb refers to Fabric Core which definitely still has these classes.

@phkuo phkuo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!!

@dzearing

Copy link
Copy Markdown
Member

[ssr-tests 16:59:07] Starting: webpack
(undefined) ../fabric-website/src/pages/PageStyles.module.scss
Module parse failed: Unexpected character '@' (1:0)
You may need an appropriate loader to handle this file type.

@Jahnp Jahnp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Had one comment about leaving the URL for the brand-icons page for a later PR. Otherwise, everything looks great.

{
title: 'Brand icons',
url: '#/styles/brand-icons',
url: '#/styles/brandicons',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Discussed in person--while it does indeed make sense for these to be consistent, doing this change will also break existing links. There may be a better way to handle a change/transition like this that won't result in a break--but we should probably look at doing that in a separate PR. That said--thanks for catching this!

@lynamemi lynamemi merged commit 8b16252 into microsoft:master Mar 13, 2018
@lynamemi lynamemi deleted the show-theme-generator branch March 13, 2018 21:51
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 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.

4 participants