added danger red color to text/border when no description

[kennyhuynh125]

Fixes #670 .

Added the danger red to the color of the text and border of the div where the No description text is showing up.

[lpatmo]

Thanks for taking a look at this, Kenny! :)

Some feedback in the comments.

[lpatmo]

Feedback:

• Instead of applying the color rule to .well p, let's give the div.well that contains the "Your study group doesn't have a description yet" its own CSS class -- maybe call it .warning -- and apply the styles there in _study_groups.scss?

This way we can ensure that other text won't be affected, like:

The other thing I noticed is that even though apply a border color, it's not showing up. I believe you're missing a solid argument; it needs to be 1px solid $danger-red.

Hope this feedback helps!

[kennyhuynh125]

Hi @lpatmo,

Thank you for the feedback! I just made some changes, hopefully it works now! Please take a look! :)

[lpatmo]

@kennyhuynh125 That's a good step... but are you sure you wanted to make all .well .jumbotron border red? :)

One possibility is putting the .warning class on the div itself, and then doing something like:

  .warning {
   border: 1px solid $danger-red;
    p { 
      color: $danger-red;
     }
   }

Note: I have not tested the above; might be worth previewing it in the console or running the app. Let me know if you have questions.

P.S. Let me also know if you're having trouble getting the app to run, or if any of the instructions on docs.codebuddies.org are confusing. Installing meteor shouldn't take too long, but the first time you hit meteor --settings settings-development.json it takes a few minutes.

[kennyhuynh125]

Hi @lpatmo

Thank you for the feedback! I wasn't sure whether well was a class used only in that template, seems like it is used elsewhere as well. I added an ID to that div so that the red border is specific only to that div.

[kennyhuynh125]

Oops! i actually didnt see the second half of your feedback :(, i believe the id will still work, but your method sounds better. i will try and work on this more tomorrow!

[lpatmo]

@kennyhuynh125 Yeah, an id will technically work, but the reason I suggested naming the class warning and applying it to the div is so that in future, we'd be able to reuse the class whenever we had a similar well we wanted to draw attention to. :) Thanks for working on this!

[kennyhuynh125]

Hey @lpatmo

I am having trouble with getting the app to run, been getting an error. I updated meteor with a patch to see if it works again.

I tried doing the CSS on Codepen and realized that your way will include a border within the p element as well (assuming that p also has a class called warning). I was wondering if it would work if I give both the div and the p element .warning but use the other classes to style it, something like...
.well .warning { border: 1px solid $danger-red; } .text-center .warning { color: $danger-red; }

[lpatmo]

Sorry for the slow followup, @kennyhuynh125! Pushed up some changes (https://github.com/codebuddiesdotorg/codebuddies/pull/672/files#diff-00f4fbcbe51fb5f21b5fa99939627f80) that simplifies things; in short, if you apply a class to the wrapping div, you can style the border and also the text inside it too. :) Let me know if you have any questions. Thanks again for your time on this!