Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Fix Bug 1284584: Standardize learn-box #3900

Merged
merged 1 commit into from Jul 26, 2016
Merged

Conversation

stephaniehobson
Copy link
Contributor

@stephaniehobson stephaniehobson commented Jul 5, 2016

Made .learn-box use the .properties styles but made it orange.

Removed old learn-box styles from customcss.styl

Updated .standard-table definition to exclude learn-boxes because .learn-box.standard-table is a very common combination.

Testing pages:
https://developer-local.allizom.org/en-US/Learn/HTML/Howto/Use_JavaScript_within_a_webpage
https://developer-local.allizom.org/en-US/Learn/HTML/Howto/Add_a_hit_map_on_top_of_an_image
https://developer-local.allizom.org/en-US/Learn/CSS/Using_CSS_in_a_web_page
https://developer-local.allizom.org/en-US/Learn/CSS/Basic_text_styling_in_CSS

When testing also edit a table to remove the standard-table class and make sure it displays properly.

@codecov-io
Copy link

codecov-io commented Jul 5, 2016

Current coverage is 85.93%

Merging #3900 into master will not change coverage

@@             master      #3900   diff @@
==========================================
  Files           144        144          
  Lines          8603       8603          
  Methods           0          0          
  Messages          0          0          
  Branches       1137       1137          
==========================================
  Hits           7393       7393          
  Misses          975        975          
  Partials        235        235          

Powered by Codecov. Last updated by 0b8b01c...85dba87

background-color: #fce1ce;
}

/* small screen improvments to table display, only need to apply to .properties and .learn-box

Choose a reason for hiding this comment

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

"improvments" > "improvements"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only things I can spell correctly reliably are CSS properties ;)

@schalkneethling
Copy link

@stephaniehobson So, I did:

(env) vagrant@developer-local:~/src$ sudo mysql -uroot -pkuma kuma < sanitized_devmo.sql
Warning: Using a password on the command line interface can be insecure.
(env) vagrant@developer-local:~/src$ foreman start

But when I try to load the pages you mentioned I get a 404. Is there something else I am missing?

@stephaniehobson
Copy link
Contributor Author

stephaniehobson commented Jul 6, 2016

Hm, that should be it and you should have the same data as I do :/ Can you follow any links off the home page?

Made .learn-box use the .properties styles but made it orange.

Removed old learn-box styles from customcss.styl

Updated .standard-table definition to exclude learn-boxes because .learn-box.standard-table is a very common combination.
@stephaniehobson
Copy link
Contributor Author

If you don't have those pages locally you can copy them over from prod. Or find other pages to copy over from prod. All pages on prod using the learn box class.

You can create new pages by visiting the new page page.

Updated my spelling error.

@schalkneethling
Copy link

Works as expected. The only concern I have is with the contrast, especially the link color on the peach like background color:

screen shot 2016-07-12 at 10 31 57

@stephaniehobson Thoughts?

@stephaniehobson
Copy link
Contributor Author

I really want to fix the contrast problems on MDN but I think that's a separate PR. This is a combination that is already in production in other places and we have bug 924583 for the contrast problems.

Merging since it works as expected.

@stephaniehobson stephaniehobson merged commit 0971c7c into master Jul 26, 2016
@stephaniehobson stephaniehobson deleted the 1284584-learning-box branch July 27, 2016 19:48
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.

None yet

3 participants