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

Review and integrate open190: new .loading CSS class #191

Merged
merged 2 commits into from
Oct 19, 2015
Merged

Conversation

charlesh88
Copy link
Contributor

open #190
CSS for revised .loading class;
Commented out/removed old .loading styles;
Theme constants for loading colors added to theme files;
See notes in #190 for usage.

Author checklist:

  • Changes address original issue? Y
  • Unit tests included and/or updated with changes? N/A
  • Command line build passes? N/A
  • Expect to pass code review? Y

open #190
CSS for revised .loading class;
Commented out/removed old .loading styles;
Theme constants for loading colors added
to theme files;
@larkin
Copy link
Contributor

larkin commented Oct 16, 2015

@charlesh88 do we have a place for style docs like this? Thinking of an analogue to the bootstrap docs, something that shows usage of css and html and serves as documentation of it.

@@ -227,7 +227,8 @@

.load-icon {
position: relative;
&.loading {

/* &.loading {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of commenting out, can we just remove this? Git history will allow it to be recovered if we need it later.

@larkin
Copy link
Contributor

larkin commented Oct 16, 2015

@charlesh88 Instead of commenting out unused scss, can you remove it instead? It can be restored from git history if we need it at a later date.

@larkin larkin assigned charlesh88 and unassigned larkin Oct 16, 2015
@larkin larkin modified the milestones: Asimov, Ballard Oct 19, 2015
open #190
Removed old .loading styles previously
commented out;
@charlesh88
Copy link
Contributor Author

Per suggestion, removed classes previously commented out and pushed to github.

@charlesh88
Copy link
Contributor Author

HTML/CSS usage is documented in the Style Guide, toward the end: https://drive.google.com/a/nasa.gov/file/d/0B0v27OM1JkSeRTg0bzE1Q0hDOEE/view?usp=sharing

That said, this doc definitely needs to be updated and I'll add a task for this in the current sprint.

@larkin larkin assigned larkin and unassigned charlesh88 Oct 19, 2015
@larkin larkin merged commit fb0ce1e into master Oct 19, 2015
@larkin
Copy link
Contributor

larkin commented Oct 19, 2015

Review Checklist

  1. Changes appear to address issue? Y
  2. Appropriate unit tests included? N/A, style only changes
  3. Code style and in-line documentation are appropriate? Y
  4. Commit messages meet standards? Y
  5. Project-specific information isolated to appropriate branches? Y

Integration Checklist

  1. Automated build passing after merge? Y
  2. No merge conflicts (or resolution trivial/obvious)? Y
  3. All master branches up-to-date after merge? Y
  4. Project-specific information isolated to appropriate branches? Y

@larkin larkin deleted the open190 branch October 19, 2015 18:37
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.

2 participants