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

Remove inline styles #456

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 4 additions & 0 deletions less/components/enrichment.less
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@

div.value {}
}

.col-xs-2{
white-space: nowrap;
}
}

.popover-content {
Expand Down
12 changes: 12 additions & 0 deletions less/components/icons.less
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,15 @@
.icon-2x {
.icon(2em)
}


#icons {
position :absolute;
width :0;
height :0;
}


.icon.icon-globe{
fill: #939393;
}
4 changes: 4 additions & 0 deletions less/components/querybuilder.less
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ div.column-container {
}

.model-browser {

.icon {
fill: darken(silver,10%);
}
Expand Down Expand Up @@ -308,6 +309,7 @@ div.column-container {
// padding-left: 0.2em;
border: 1px solid #d5d5d5;
border-width: 0 0 1px 1px;
white-space: nowrap;

&.haschildren {
border-bottom: 0;
Expand Down Expand Up @@ -395,6 +397,7 @@ div.column-container {
.qb-label {
padding-left: 0;
white-space: nowrap;
margin-left: 5px;
Copy link
Member

Choose a reason for hiding this comment

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

.qb-label is also used for query builder attributes, where we don't want the margin-left (see src/cljs/bluegenes/pages/querybuilder/views.cljs:83).
Maybe split it into .qb-attribute-label where the margin-left is the only difference, changing the attributes to use this class instead?

}

.icon-blank::before,
Expand Down Expand Up @@ -549,6 +552,7 @@ div.column-container {
background-color: #c8ced0;
color: #666;
border-radius: 3px;
margin-left: 5px;
}

.label-no-results {
Expand Down
4 changes: 4 additions & 0 deletions less/components/regions.less
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@
.allresults {
display: flex;
flex-direction: column;
.feature {
Copy link
Member

Choose a reason for hiding this comment

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

Excellent, this is what I want to see more of [=

overflow-wrap: break-word;
}

}

.results {
Expand Down
1 change: 1 addition & 0 deletions less/im-tables-overrides.less
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
.im-table-toolbar label {
.dark-text;
}

}

.results-and-enrichment {
Expand Down
5 changes: 5 additions & 0 deletions less/overrides.less
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,8 @@ input.form-control:focus {
label {
color: @body-foreground-color;
}

.col-xs-3{
font-size :0.8em;
}

24 changes: 23 additions & 1 deletion less/site.less
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ label {
overflow: visible;
margin-left: calc(100vw - 100%);
margin-right: 0;
}
}


.loading {
display: flex;
Expand Down Expand Up @@ -353,6 +354,12 @@ button.btn, button.cta {
}
}

Copy link
Member

Choose a reason for hiding this comment

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

I don't think many of these selectors added to site.less make much sense as global styles.

Each "component" generally have their own less source file. In the case of .container-fluid.results, you could add this styling to the less/components/results.less file instead.

Could you go through the selectors you added to site.less and see if there exist a more suitable less file to place them? There's usually a top-level class name that these page-specific styles will be nested under. Don't hesitate to create new class names if you feel this is necessary.

.btn.btn-raised.dropdown-toggle
{
text-transform: none;
white-space: normal;
}

.dropdown-mixed-content {
//mixin used for list / lookup/ = / > / < lookup dropdown
//(probably other places too)
Expand Down Expand Up @@ -399,13 +406,28 @@ button.btn, button.cta {
}
}

.container-fluid.results{
width: 100%;
}

.icon.icon-document-list {
margin-left: 0px;}

.relative.im-table{
background-color: white;
}
.no-gutter {
margin-right: 0;
margin-left: 0;
}

.description-div{
background-color: #D2CEBF;
padding: 10px;
overflow: auto;
}


.no-gutter > [class*="col-"] {
padding-right: 5px;
padding-left: 5px;
Expand Down
1 change: 1 addition & 0 deletions less/variables.less
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
.icon {
color: @cta-color-text;
fill: @cta-color-text;
position: absolute;
Copy link
Member

Choose a reason for hiding this comment

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

This seems to break the positioning of icons for some buttons elsewhere.
Could you instead add a more specific rule to templates.less, under .constraint-container?

}
}

Expand Down
15 changes: 6 additions & 9 deletions src/cljs/bluegenes/components/icons.cljs

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions src/cljs/bluegenes/components/ui/alerts.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@
[:span.markup markup]
[:span.controls
[:button.btn.btn-default.btn-xs.btn-raised
{:on-click (fn [] (dispatch [:messages/remove id]))
:style {:margin 0}} "X"]]])}))
Copy link
Member

Choose a reason for hiding this comment

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

I can't see that margin: 0 is added back via the CSS, and in this case it makes a visual difference.
Could you add it to alerts.less?

{:on-click (fn [] (dispatch [:messages/remove id]))} "X"]]])}))

(defn messages
"Creates a message bar on the bottom of the screen"
Expand Down
4 changes: 1 addition & 3 deletions src/cljs/bluegenes/components/ui/constraint.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@
((or on-change-operator on-change) {:code code :path path :value (cond-> value (seq? value) first) :op op})))]]
[:div.col-sm-8
[:div
{:style {:position "relative"}}
[constraint-text-input
:model model
:value value
Expand All @@ -276,8 +275,7 @@
((or on-blur on-change) {:path path :value val :op op :code code})))]
(when on-remove
[:svg.icon.icon-bin
{:style {:position "absolute"}
:on-click (fn [op] (on-remove {:path path :value value :op op}))}
{:on-click (fn [op] (on-remove {:path path :value value :op op}))}
[:use {:xlinkHref "#icon-bin"}]])]]]]
#_[:div.constraint-component
[:div.input-group.constraint-input
Expand Down
6 changes: 2 additions & 4 deletions src/cljs/bluegenes/components/ui/list_dropdown.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,16 @@
filter-fn (apply every-pred [text-filter type-filter])
filtered-lists (filter filter-fn lists)]
[:div.dropdown
[:button.btn.btn-raised.btn-default.dropdown-toggle
[:button.btn.btn-raised.dropdown-toggle
{:disabled disabled
:style {:text-transform "none"
:white-space "normal"}
:data-toggle "dropdown"}
(str (or value "Choose a list") " ") [:span.caret]]
[:div.dropdown-menu.dropdown-mixed-content
(if (some? (not-empty lists))
[:div.container-fluid
[text-filter-form text-filter-atom]
[:div.col-md-6
[:h4 [:svg.icon.icon-history [:use {:xlinkHref "#icon-history"}]] " Recently Created"]
[:h4 [:svg.icon.icon-history [:use {:xlinkHref "#icon-history"}]] " Recently Created"]
[im-lists
:lists (take 5 (sort-by :timestamp filtered-lists))
:on-change on-change]]
Expand Down
3 changes: 1 addition & 2 deletions src/cljs/bluegenes/events/registry.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@
(not (contains? registry current-mine))
{:db (assoc db-with-registry :current-mine :default)
:dispatch [:messages/add
{:markup [:span (str "Your mine has been changed to the default as your selected mine '" (name current-mine) "' was not present in the registry.")]
:style "warning"}]}
Copy link
Member

Choose a reason for hiding this comment

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

Could you revert this change? It's by chance called :style but not an inline style in this instance.

{:markup [:span (str "Your mine has been changed to the default as your selected mine '" (name current-mine) "' was not present in the registry.")]}]}
;; Fill in the mine details if it's missing.
;; (This happens when we use a registry mine.)
(nil? (get-in db-with-registry [:mines current-mine]))
Expand Down
5 changes: 3 additions & 2 deletions src/cljs/bluegenes/pages/mymine/views/mymine.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
(defn ico []
(fn [file-type]
(case file-type
"list" [:svg.icon.icon-document-list {:style {:margin-left 0}}
"list" [:svg.icon.icon-document-list
[:use {:xlinkHref "#icon-document-list"}]]
"tag" [:svg.icon.icon-folder [:use {:xlinkHref "#icon-folder"}]]
[:svg.icon.icon-folder [:use {:xlinkHref "#icon-spyglass"}]])))
Expand All @@ -146,7 +146,8 @@
[:a {:href (route/href ::route/list {:title (:title dets)})}
name]
(when-not authorized
[:svg.icon.icon-globe {:style {:fill "#939393"}} [:use {:xlinkHref "#icon-globe"}]])]
[:svg.icon.icon-globe
[:use {:xlinkHref "#icon-globe"}]])]
[:div.description-text
description]]
[:div.col-1 {:class (str "tag-type type-" type)} type]
Expand Down
15 changes: 6 additions & 9 deletions src/cljs/bluegenes/pages/querybuilder/views.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,10 @@
[:div.model-browser
(let [path [root-class]]
(into [:ul
[:li [:div
{:style {:white-space "nowrap"}}
[:button.btn.btn-default.btn-slim
{:on-click (fn [] (dispatch [:qb/enhance-query-add-summary-views [root-class]]))}
[:span.label-button
"Summary"]]
[:li [:div [:button.btn.btn-default.btn-slim
{:on-click (fn [] (dispatch [:qb/enhance-query-add-summary-views [root-class]]))}
[:span.label-button
"Summary"]]
[:button.btn.btn-default.btn-slim
{:on-click (fn [] (dispatch [:qb/expand-all]))}
"Expand to Selection"]
Expand All @@ -155,7 +153,7 @@
[:li.tree.haschildren
[:div.flexmex
[:span.lab {:class (if (im-path/class? model (join "." path)) "qb-class" "qb-attribute")}
[:span.qb-label {:style {:margin-left 5}} [tooltip {:on-click #(dispatch [:qb/expand-path path]) :title (str "Show " (uncamel k) " in the model browser")} [:a (uncamel k)]]]
[:span.qb-label [tooltip {:on-click #(dispatch [:qb/expand-path path]) :title (str "Show " (uncamel k) " in the model browser")} [:a (uncamel k)]]]
(when-let [s (:subclass properties)] [:span.label.label-default (uncamel s)])
[:svg.icon.icon-bin
{:on-click (if (> (count path) 1)
Expand All @@ -166,8 +164,7 @@
[:svg.icon.icon-filter {:on-click (fn [] (dispatch [:qb/enhance-query-add-constraint path]))} [:use {:xlinkHref "#icon-filter"}]]
(when-let [c (:id-count properties)]
[:span.label.label-soft
{:class (when (= 0 c) "label-no-results")
:style {:margin-left 5}} (str c " row" (when (not= c 1) "s"))])]]
{:class (when (= 0 c) "label-no-results")} (str c " row" (when (not= c 1) "s"))])]]
(when-let [constraints (:constraints properties)]
(into [:ul.tree.banana]
(map-indexed (fn [idx con]
Expand Down
3 changes: 1 addition & 2 deletions src/cljs/bluegenes/pages/regions/results.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@
:type class
:id objectId})}
[:div.grid-3_xs-3.single-feature
[:div.col {:style {:word-wrap "break-word"}}
primaryIdentifier]
[:div.col.feature primaryIdentifier]
[:div.col the-type]
[:div.col (str
(get-in chromosomeLocation [:locatedOn :primaryIdentifier])
Expand Down
3 changes: 1 addition & 2 deletions src/cljs/bluegenes/pages/reportpage/views.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
[:h3 (str title (when result-count (str " (" result-count ")")))]
(if (= 0 result-count)
[:div "No Results"]
[:div {:style {:background-color "white"}}
[im-table/main loc]])])))})))
[:div [im-table/main loc]])])))})))

(defn strip-class [s]
(clojure.string/join "." (drop 1 (clojure.string/split s "."))))
Expand Down
4 changes: 2 additions & 2 deletions src/cljs/bluegenes/pages/results/enrichment/views.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@
:title
identifier)}]))}
description]}])]
[:div.col-xs-3 [:span {:style {:font-size "0.8em"}} (.toExponential p-value 6)]]]]])))
[:div.col-xs-3 [:span
(.toExponential p-value 6)]]]]])))

(defn has-text?
"Return true if a label contains a string"
Expand All @@ -94,7 +95,6 @@
[:div.container-fluid
[:div.row
[:div.col-xs-2
{:style {:white-space "nowrap"}}
[:input
{:type "checkbox"
:checked selected?
Expand Down
3 changes: 0 additions & 3 deletions src/cljs/bluegenes/pages/results/views.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,14 @@
(if @are-there-results?
;;show results
[:div.container-fluid.results
{:style {:width "100%"}}
[:div.row
[:div.col-sm-2
[query-history]]
[:div.col-sm-7
[:div
{:style {:background-color "white"}}
[tables/main [:results :table]]
(when (> (count description) 0)
[:div.description-div
{:style {:background-color "#D2CEBF" :padding "10px" :overflow "auto"}}
[:b "Description: "]
description])]
[:div
Expand Down