Add panel styles for widgets #175

[atcraigwatson]

Completes #175

Added Bootstrap markup to the default widget to use the Bootstrap Panel styles.

[atcraigwatson]

@salcode

Well I have had a bash at it, I hope you don't mind. This issue seemed the most logical to start on as it follows the direction of the theme.

[salcode]

@atcraigwatson

Way to go! A first PR is an awesome thing.

Now this is the part of PRs that isn't quite as fun, where the repo owner is really particular about things.

package-lock.json

We definitely need to do something regarding this file but I'd rather see it handled in a different PR. I've opened #179 on this topic.

If you want to add a new commit where you delete the file, I'll squash them together when I merge this PR (when, not if).

package.json

I realize this is a really small typo correction, eliminating the spaces before the colons but I've found the keeping even small changes like this segmented make my life easier. For example, if we need to revert a commit, we don't want to revert a side change like this, as well. I've opened #180 to address this.

Removing box-shadow and other CSS changes

I haven't had a chance but I want to check out these CSS changes on my local install. They seem to make sense but I'd like to see them in action.

Summary

It is super rare that I've ever had my first draft of a PR merged. You did an awesome job on this and have a great deal to be proud of. If you want to make the first two changes to your cw/widget-panel-styles-175 branch, when you push the branch the PR will be updated.

I'll have a chance to check out the CSS sometime in the next 24 hours.

Thank you for the PR.

[atcraigwatson]

Hi @salcode

I am kind of pleased there are things to work on with the PR, it makes for even better experience when working with them and having to revisit code and make updates.

Let's have a go at making these changes and making the commit.

[salcode]

@atcraigwatson

Your updates with removing files are perfect, nice work.

I had a chance to preview the CSS and I have a couple of thoughts.

Box Shadow

I understand why you dropped the box shadow (and ultimately it may look better without) but I'd prefer to keep the default rendering as part of this theme ( maybe everyone who builds a theme with it will remove the drop shadow 😀 , but I think it makes sense to stick with the default ). This also let's us avoid having to use an !important property.

For reference, here are screenshots with and without the box shadow.

With Box Shadow

Without Box Shadow

Unused Variables

Since your PR involves removing some of the widget CSS lines (which I love by the way, my favorite changes are removals rather than additions), I think these three variables from css/sass/supporting/variables-bootstrap-genesis.scss are no longer used.

• $widgetFontColor
• $widgetBgColor
• $widgetBorderColor

Would you please remove these three variables as part of your PR.

Summary

If you would be so kind as to make these two changes, we should be good to go. Thanks for all your work on this.

[atcraigwatson]

@salcode

The box shadow is something I was unsure on myself to be honest, the more I worked with the project and got a feel for it I became aware that editing a default style of Bootstrap was the wrong way to go.

My bad on the variable front, that is something I need to be aware of when editing code in this way, checking what is left behind and what it effects.

I can't thank you enough for being as supportive as you have been while I work through my fist PR!! I have learned a lot in a very short space of time.

I will make these updates as separate commits, then on to making my blog 😄

[salcode]

@atcraigwatson

Sounds like a great plan.

It is my pleasure working with you. Other than very simple PRs (like correcting a typo), some back-and-forth is always to be expected.

You're doing a great job keeping your commits focused on exactly one thing and the PR as a whole focused on this one change. I'll look forward to getting this PR merged. 👍

Thanks.

[atcraigwatson]

Would it be best practice to delete the branch locally and push changes?

[salcode]

Once a branch is merged, you can delete both your local copy and your copy at GitHub

Then if you have this project setup as your upstream, you can update your develop branch by running

git pull upstream develop

which should bring you local copy of the develop up-to-date with the version here.