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: whitespace changes and reformatting of css files #1872

Merged
merged 2 commits into from
Oct 20, 2020

Conversation

ialarmedalien
Copy link
Collaborator

@ialarmedalien ialarmedalien commented Oct 16, 2020

Description of PR purpose/changes

Ran the stylelint css formatter/linter over the codebase to enforce whitespace and formatting; stylelint is one of the code quality checks run by Codacy.

In addition to formatting the css, stylelint also identified a number of empty stanzas, which I removed.

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.

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 - N/A
  • 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
  • Any dependent changes have been merged and published in downstream modules - N/A
  • I have run Black and Flake8 on changed Python Code manually or with git precommit (and the travis build passes) - N/A

Updating Version and Release Notes (if applicable)

@coveralls
Copy link

coveralls commented Oct 16, 2020

Coverage Status

Coverage increased (+0.006%) to 16.967% when pulling 2456e40 on DATAUP-246_css_to_sass-whitespace into c3ce86f on truss.

@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2020

SonarCloud Quality Gate failed.

Bug E 11 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

margin-left: 10px;

/* Not a link anymore, so we disable for now
cursor: pointer; */
Copy link
Contributor

Choose a reason for hiding this comment

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

in the spirit of cleaning up comments should we get rid of this as well?

margin: 2px;
margin-left: 10px;

/* cursor:pointer; */
Copy link
Contributor

Choose a reason for hiding this comment

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

moar comments

color: #FFF;
background-color: #FFF;
text-shadow: none;
/* like mayo on wonderbread */
Copy link
Contributor

Choose a reason for hiding this comment

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

lol

/*background: #eeeded;*/
margin: 0 auto;

/* background: #eeeded; */
Copy link
Contributor

Choose a reason for hiding this comment

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

another rando comment

font-size: 1.1em;

/* max-height: 2.6em;
overflow:hidden; */
Copy link
Contributor

Choose a reason for hiding this comment

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

some more commented out cruft

font-size: 2em;

/* max-height: 2.6em;
overflow:hidden; */
Copy link
Contributor

Choose a reason for hiding this comment

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

crufted code

Comment on lines +3262 to +3265
/* overflow-x: hidden;
overflow-y: auto; */

/* height: 830px; */
Copy link
Contributor

Choose a reason for hiding this comment

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

chunk of cruft

display: inline-block;

/* background-color: #2196F3;
color: #BBDEFB; */
Copy link
Contributor

Choose a reason for hiding this comment

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

more crufty code

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.

Tried reviewing as best I could. Running locally everything looks normal (or as best as I can test with prod services and staging down). Looks like there's some comments trailing in a few of these files that we could clean up (if you still have the energy after this monster) flagged a few that I saw.

Comment on lines +3496 to +3501

/* .container { */

/* max-width: 2000px !important; */

/* } */
Copy link
Contributor

Choose a reason for hiding this comment

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

trash it!

.nav > li > a:focus {
text-decoration: none;

/* background-color: #fff !important; */
Copy link
Contributor

Choose a reason for hiding this comment

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

can we chuck it?

/*margin: 15px 7px 0 0;*/
display: inline-block;
padding: 0 15px 0 0;
/* margin: 15px 7px 0 0; */
Copy link
Contributor

Choose a reason for hiding this comment

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

here too

Comment on lines +95 to +97
/* #select-box tr :hover {
background-color: #eee;
}*/
} */
Copy link
Contributor

Choose a reason for hiding this comment

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

another one we can get rid of?

}

/*div::-webkit-scrollbar {
/* div::-webkit-scrollbar {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get rid of this chunk too? 130-145 (tried multi-line comment and for some reason it wouldn't)

}

header[role="banner"] {
/* overflow: auto;
/* overflow: auto;
padding: 7px 0 2px 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

this chunk as well and just below have some comments we can get rid of

nav[role="navigation"] ul {
margin: 1.2em 0 0.5em 0;

/* text-align: center; */
Copy link
Contributor

Choose a reason for hiding this comment

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

dee-lete

position: absolute;
top: 55px;
bottom: 5;
/* width: 1170px; */
Copy link
Contributor

Choose a reason for hiding this comment

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

commentttt

Comment on lines +309 to +312
/* float: left; */

/* max-width: 1170px;
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

junk it

}

/*.left-pane {
/* .left-pane {
font-size: 14px;
Copy link
Contributor

Choose a reason for hiding this comment

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

some more comments we could remove here and generally in this file

Copy link
Member

@briehl briehl left a comment

Choose a reason for hiding this comment

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

Looks fine to me. This one's all about reformatting, and the other PR (#1879) removes crufty rules and commented out things, right?

@bio-boris
Copy link
Contributor

@ialarmedalien I tested this out locally, worked fine. It's up to you if you'd like to merge it in or delete more cruft this pass.

@bio-boris
Copy link
Contributor

@ialarmedalien Also there are a bunch of sonarcloud issues which are an easy fix, adding a default font family

@ialarmedalien
Copy link
Collaborator Author

@bio-boris I'll add them in a later PR. This one is almost entirely whitespace changes.

@ialarmedalien ialarmedalien merged commit ec3c4af into truss Oct 20, 2020
@ialarmedalien ialarmedalien deleted the DATAUP-246_css_to_sass-whitespace branch October 20, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants