Skip to content

[Website] Update incorrect documentation on variables#1333

Merged
kategarklavs merged 1 commit into
stagingfrom
ms-update_vars_documentation
Jul 18, 2016
Merged

[Website] Update incorrect documentation on variables#1333
kategarklavs merged 1 commit into
stagingfrom
ms-update_vars_documentation

Conversation

@msecret

@msecret msecret commented Jul 18, 2016

Copy link
Copy Markdown
Contributor

Description

The documentation on this is now incorrect as variables.scss is no longer being imported so is not technically where these things are defined.

Additional information

Also, the way this was previously worded, it sounded like it was suggesting people to modify the src files of the standards which I've been working hard to stop from happening because it's a terrible practice.

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

The documentation on this is now incorrect as variables.scss is no longer being imported so is not technically where these things are defined.

Also, the way this was previously worded, it sounded like it was suggesting people to modify the src files of the standards which I've been working hard to stop from happening because it's a terrible practice.
@maya

maya commented Jul 18, 2016

Copy link
Copy Markdown
Contributor

This looks good, the only issue I have is the "importing it before uswds in Sass" bit. I think it could be rewritten a bit. Maybe say:

add your custom variables.scss import before the Draft Standards all import in your main.scss file.

or

import your custom variables.scss before importing the Draft Standards all Sass in your main.scss file.

Possibly addresses #1334 although I wonder if we need to add a section in https://standards.usa.gov/getting-started/developers/

@kategarklavs

Copy link
Copy Markdown
Contributor

@maya This looks good to me. Also, I'm happy to chat more about adding a new section to the Getting Started section, if you think that this is necessary.

@kategarklavs kategarklavs merged commit ec27b28 into staging Jul 18, 2016
@maya

maya commented Jul 18, 2016

Copy link
Copy Markdown
Contributor

@kategarklavs cool, and just checking "changing variable values and importing it before uswds in Sass." sounds OK to you?

@kategarklavs

Copy link
Copy Markdown
Contributor

@maya Hello again, and apologies for not being clear before — that sounds good to me 👍

@maya maya deleted the ms-update_vars_documentation branch July 18, 2016 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants