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

DATAUP-246: css to sass: css cleanup, part 1 #1867

Merged
merged 2 commits into from
Oct 16, 2020

Conversation

ialarmedalien
Copy link
Collaborator

@ialarmedalien ialarmedalien commented Oct 15, 2020

Description of PR purpose/changes

Cleaning up the existing CSS files prior to conversion to SCSS.

Splitting this work up into small chunks to make it less of a nightmare to PR.

First commit:

  • there was a small amount of css in some appCell-specific files; since it was namespaced nicely, I moved consolidated it all into the existing appCell.css file. Fixed up errors in appCell.css and reworked the css to combine duplicated statements.

Second commit:

  • fixed more errors in existing css files; still have a couple more to do, but these will go in a separate PR.

Jira Ticket / Issue

https://kbase-jira.atlassian.net/browse/DATAUP-246

Testing Instructions

  • Details for how to test the PR:
  • Tests pass in travis and locally
  • Changes available by spinning up a local narrative and clicking around to look at parts of the UI.

Dev Checklist:

  • My code follows the guidelines at https://sites.google.com/truss.works/kbasetruss/development
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation - N/A
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works - N/A
  • New and existing unit tests pass locally with my changes

Updating Version and Release Notes (if applicable)

N/A

Author : erik
*/

.kb-advanced-view-cell .kb-panel-container[data-element=parameters-group] > div.panel-collapse > .panel-body {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved this into appCell.css

'kb_service/utils',
'kb_service/client/workspace',
'css!kbase/css/appCell.css',
'css!./styles/main.css',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

consolidated the styles in ./styles/main.css into kbase/css/appCell.css and removed /styles/main.css

@@ -26,7 +26,6 @@ define([
'kb_service/client/workspace',
'./appCell',
'css!kbase/css/appCell.css',
'css!./styles/main.css',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

file has no css content => deleted

@@ -1,419 +0,0 @@
/* Wrapper class
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved all these styles into appCell.css. The majority mimic styles that were already in that file.

@@ -7,24 +7,30 @@
font-family: 'Oxygen', sans-serif;
}

.kb-app-cell .kb-app-warning {
.kb-app-cell .kb-app-warning,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The majority of new content in this file comes from editor cell css file, and mimics the existing .kb-app-cell styles.

}
*/

.btn.kb-flat-btn:hover {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

overwritten in next declaration. doh!

Comment on lines -60 to -61
text-decoration-style: solid;
text-decoration-color: red;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

invalid


/* margin left doesn't really seeem to play well */

.btn-toolbar {
margin-left: 0;
padding-left: -5px;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cannot use negative units for padding

@@ -1,181 +0,0 @@
/* Wrapper class
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this file is unused

color: #FFF;
background-color: rgb(209, 82, 65);
border-bottom: 6px rgb(209, 82, 65) solid;
margin-bottom: 0;
}

.btn.kb-app-cell-btn.btn-danger.active:hover {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

repeated two stanzas below

Comment on lines 267 to 278
margin-left: 0px;
margin-right: 4px;
font-family: "FontAwesome";
font-style: normal;
font-weight: normal;
font-size: 90%;
width: 12px;
color: silver;
line-height: 1;
vertical-align: baseline;
content: "\f078 ";
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

identical to .kb-app-cell .panel-title > [data-toggle="collapse"] below

@coveralls
Copy link

coveralls commented Oct 15, 2020

Coverage Status

Coverage remained the same at 16.961% when pulling 6ae2c7e on DATAUP-246_css_to_sass-css_cleanup into d78cc01 on truss.

Copy link
Contributor

@leia-sefkin leia-sefkin left a comment

Choose a reason for hiding this comment

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

Oh dang what a satisfying cleanup pass already! So many xcolor and xmargin and xelement_that_somedev_was_too_lazy_to_properly_remove :P

Thanks for all the comments it helps to review all of these changes. Looks good overall to me

@ialarmedalien ialarmedalien force-pushed the DATAUP-246_css_to_sass-css_cleanup branch from dfe05ab to d7a2779 Compare October 15, 2020 23:23
…n/static/kbase/css/appCell.css`

Validate appCell.css and fix errors
…pHelper.css, buttons.css, contigBrowserStyles.css, kbaseJobLog.css, kbaseNotify.css, kbaseStylesheet.css, and kbaseTour.css. Also fixed errors in kbase-extension/static/kbase/custom/custom.css.
@ialarmedalien ialarmedalien force-pushed the DATAUP-246_css_to_sass-css_cleanup branch from d7a2779 to 6486a6a Compare October 15, 2020 23:24
@sonarcloud
Copy link

sonarcloud bot commented Oct 15, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ialarmedalien ialarmedalien merged commit 6c45a0b into truss Oct 16, 2020
@ialarmedalien ialarmedalien deleted the DATAUP-246_css_to_sass-css_cleanup branch October 16, 2020 13:18
@ialarmedalien ialarmedalien restored the DATAUP-246_css_to_sass-css_cleanup branch October 16, 2020 18:15
@ialarmedalien ialarmedalien deleted the DATAUP-246_css_to_sass-css_cleanup branch November 25, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants