Skip to content

Issue #89: Style/SCSS review #91

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.

Sign up for GitHub

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

Merged
andrew-c-tran merged 3 commits into from
Feb 27, 2018
Merged

Issue #89: Style/SCSS review #91

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a74064
Using Bootstrap color variables. Reorginzaton of Sass partials. Pulli…
aManNamedJed Feb 26, 2018
f9a02d1
Removed Bootstrap css file from head
aManNamedJed Feb 26, 2018
cd0c107
Add column class to the globals partial
aManNamedJed Feb 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
<html>
<head>
<title>DDEV UI</title>
<link href="css/bootstrap.min.css" rel="stylesheet">
<link rel="stylesheet" href="css/font-awesome.min.css">
<script>if (typeof module === 'object') {window.module = module; module = undefined;}</script>
<script src="js/src/jquery-3.2.1.min.js"></script>
Expand All @@ -11,13 +10,13 @@
<script>if (window.module) module = window.module;</script>
</head>
<body style="display: none;">
<nav class="navbar navbar-toggleable-md navbar-inverse fixed-top bg-inverse">
<nav class="navbar navbar-toggleable-md navbar-inverse bg-inverse">
<img src="img/ddev_last.png" class="ddev-logo" />
<input class='directory-select' type="file"
webkitdirectory />
</nav>

<div class="container">
<div class="container-fluid">
<!-- Example row of columns -->
<div class="row card-container">
</div>
Expand Down
2 changes: 1 addition & 1 deletion js/src/bootstrap-modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require('../../scss/modals.scss');
require('../../scss/components/modals.scss');
//Markup Generators
/**
* TEMPLATE - generates markup for a bootstrap modal
Expand Down
8 changes: 4 additions & 4 deletions js/src/site-cards.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
var ddevShell = require('./ddev-shell');
var electron = require('electron');
require('../../scss/card.scss');
require('../../scss/components/card.scss');

/**
* TEMPLATE - generates markup for the placeholder "add/create site" card
* @returns {string} - markup for add/create site card
*/
function createAddCard(){
var markup = `<div class="column col-lg-3 col-md-4 col-sm-4">
<div class="card add">
<div class="ddev-card card add">
<div class="card-header">
<h2><a href="#">Add/Create Project</a></h2>
</div>
Expand All @@ -34,14 +34,14 @@ function createAddCard(){
*/
function createCard(site) {
var markup = `<div class="column col-lg-3 col-md-4 col-sm-4 ` + site.status + `" data-path="` + site.approot + `" data-sitename="` + site.name + `">
<div class="card">
<div class="ddev-card card">
<div class="card-header">
<h2><a href="#" class="open-site" data-url="` + site.httpurl + `">` + site.name + `</a></h2>
</div>
<div class="card-body">
<a href="#" class="open-site" data-url="` + site.httpurl + `">
<div class="site-icon-container">
<img class="site-icon" src="img/` + site.type + `.png" />
<img class="site-icon" src="img/` + site.type + `.png" />
</div>
<div class="card-status">
<div>` + site.status + `</div>
Expand Down
23 changes: 6 additions & 17 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"webpack-dev-server": "^2.11.0"
},
"dependencies": {
"bootstrap": "^4.0.0",
Copy link
Contributor

@andrew-c-tran andrew-c-tran Feb 26, 2018

Choose a reason for hiding this comment

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

nice! i didn't realize v4 finally went stable last month. feels like it's been in beta forever!

"compare-versions": "^3.1.0",
"electron-is-dev": "^0.3.0",
"fix-path": "^2.1.0",
Expand Down
14 changes: 14 additions & 0 deletions scss/common/globals.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
@import "../common/variables.scss";

/*
Global application wide styles and fonts
*/
body {
font-family: 'Montserrat', sans-serif;
background-color: $light;
display: block !important;
}

::-webkit-scrollbar {
display: none;
}
22 changes: 22 additions & 0 deletions scss/common/typography.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
@font-face {
font-family: 'Montserrat';
src: url('../fonts/Montserrat-ExtraLight.ttf') format('truetype');
font-weight: normal;
font-style: normal;
}

h1, h2, h3 {
color: $primary;
}

h1 {
font-size: 2rem;
}
Copy link
Contributor

@andrew-c-tran andrew-c-tran Feb 26, 2018

Choose a reason for hiding this comment

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

r/em isn't super required here since it's a packaged electron app, but i'm alright with going with rem going forward in the event we port to web.


h2 {
font-size: 1.5rem;
}

h3 {
font-size: 1.2rem;
}
13 changes: 13 additions & 0 deletions scss/common/variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
$primary: #28bde4;
$light: #c5c5c5;
$dark: #23282b;
$red: #d80000;
$grey: #878787;
$transparentDarkGrey: rgba(100,100,100,0.3);

$theme-colors: (
"primary": $primary,
"secondary": $grey,
"light": $light,
);
Copy link
Contributor

@andrew-c-tran andrew-c-tran Feb 26, 2018

Choose a reason for hiding this comment

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

theme-colors is an interesting new addition to bootstrap that I haven't played with much. Looks interesting! Though if i'm understanding this correctly we're not actually using this anywhere are we? all the references to $primary for example in all of the other files are simply referencing $primary itself and not grabbing the color from the map no?

Copy link
Contributor Author

@aManNamedJed aManNamedJed Feb 26, 2018

Choose a reason for hiding this comment

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

You're right -- I am not using the map directly, but Bootstrap actually uses the $theme-colors map to create all of the buttons, alerts, etc., so that alert-primary would then use the new color.

https://github.com/twbs/bootstrap/blob/v4-dev/scss/_alert.scss#L47-L51

Copy link
Contributor

@andrew-c-tran andrew-c-tran Feb 26, 2018

Choose a reason for hiding this comment

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

ah, interesting, that's pretty cool! in previous experiments of sass-maps, in the case of a whitelabeled/rebranded ui it became kind of a PITA to update say a primary button color, does TWBS 4 have a better way of handling that besides overriding the entire map? Meaning, can we easily override just $theme-colors('primary') in an overrides file or would we have to redefine the entire map again?

Copy link
Contributor Author

@aManNamedJed aManNamedJed Feb 26, 2018

Choose a reason for hiding this comment

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

You can declare your $theme-colors before you load in BS4's styles, and it will basically fill in the gaps for you:

https://github.com/twbs/bootstrap/blob/v4-dev/scss/_variables.scss#L74-L84

Basically this is saying "If there is no $theme-colors map, then create one now and fill it in with all this information. If there actually is a $theme-colors map, then merge it in with this data, but override the defaults provided".

It's all about that !default flag.

Copy link
Contributor Author

@aManNamedJed aManNamedJed Feb 26, 2018

Choose a reason for hiding this comment

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

Here is a thread in Bootstrap's repo where the devs/designers go back and forth on this if you're interested: twbs/bootstrap#23260


5 changes: 5 additions & 0 deletions scss/components/buttons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import "../common/variables.scss";

.btn {
cursor: pointer;
}
Copy link
Contributor

@andrew-c-tran andrew-c-tran Feb 26, 2018

Choose a reason for hiding this comment

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

Thanks for modularizing these! The intention is to co-locate the styles with the react components in the upcoming refactor, you've just saved me a good bit of work.

27 changes: 14 additions & 13 deletions scss/card.scss → scss/components/card.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
@import "../common/variables.scss";

/*
Project card styles
*/
@import "./variables.scss";

/*
Base styles
*/
.card-header, .card-footer {
background-color: $darkGrey;
.card.ddev-card .card-header, .card.ddev-card .card-footer {
background-color: $dark;
}

.card {
.card.ddev-card {
text-align: center;
color: $darkGrey;
color: $dark;
}

/*
Expand All @@ -23,7 +24,7 @@
/*
Body
*/
.card-body {
.card.ddev-card .card-body {
margin: 0;
padding: 0;
position: relative;
Expand Down Expand Up @@ -59,20 +60,20 @@
/*
Buttons and footer
*/
.running .startbtn{
display: none;
}
.card.ddev-card .btn {
margin: 5px;
}

.stopped .stopbtn{
.running .startbtn {
display: none;
}

.stopped .restartbtn{
.stopped .stopbtn {
display: none;
}

.card .btn {
margin: 5px;
.stopped .restartbtn {
display: none;
}

.add .card-footer {
Expand Down
25 changes: 25 additions & 0 deletions scss/components/footer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
@import "../common/variables.scss";

/*
Footer styles
*/
.footer {
height: 66px;
background-color: $dark;
bottom: 0;
position: fixed;
width: 100%;
}

.router-status {
line-height: 66px;
color: $white;
float: right;
}

.router-status-label {
color: $light;
float: right;
padding: 0 10px;
line-height: 66px;
}
12 changes: 12 additions & 0 deletions scss/components/header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
@import "../common/variables.scss";

/*
Header styles
*/
.navbar {
background-color: $dark;
}

.ddev-logo {
height: 50px;
}
25 changes: 16 additions & 9 deletions scss/modals.scss → scss/components/modals.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
@import "../common/variables.scss";

/*
All bootstrap modal overrides
*/
@import "./variables.scss";

/*
Header
*/
.modal-header {
background-color: $darkGrey;
background-color: $dark;
}

/*
Expand Down Expand Up @@ -55,7 +56,7 @@
margin: 10px;
}

.add-modal-section-header{
.add-modal-section-header {
border-bottom: 1px solid;
}

Expand All @@ -69,7 +70,7 @@
padding: 10px;
}

.tile img{
.tile img {
width: 100%;
}

Expand All @@ -94,28 +95,34 @@
align-items: center;
justify-content: center
}

.error-overlay {
padding: 50px;
}

.error-overlay div {
margin: 10px 0;
}

.loading-spinner {
font-size: 150px !important;
color: $lightBlue;
color: $primary;
}

.error-icon {
color: $errorRed;
color: $red;
}

.loading-text {
width: 55%;
margin-left: 20px;
color: $lightBlue;
color: $primary;
font-weight: bold;
font-size: 20px;
}

.error-text {
color: $lightBlue;
color: $primary;
font-weight: bold;
font-size: 20px;
}
Expand All @@ -125,5 +132,5 @@
*/

.modal-footer {
background-color: $darkGrey;
background-color: $dark;
}
Loading