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

#158947252 Implement feedback #41

Merged
merged 5 commits into from
Jul 12, 2018

Conversation

appcypher
Copy link
Owner

@appcypher appcypher commented Jul 12, 2018

What does this PR do?

This PR adds component tests.

Description of Task to be completed?

  • Improve all tests coverages
  • Add CONTRIBUTING.md for contributors
  • Add animations to UI
  • Make tests more specific
  • Fix responsiveness issue
  • Make app more state dependent instead of reloading window
  • Replace placeholder texts with actual content

How should this be manually tested?

To test the following features, you need to follow the following instructions here.

What are the relevant pivotal tracker stories?

158947252

- Improve all tests coverages
- Add CONTRIBUTING.md for contributors
- Add animations to UI

[Chore #158947252]
- Improve all tests coverages
- Add CONTRIBUTING.md for contributors
- Add animations to UI
- Make tests more specific
- Fix responsiveness issue
- Make app more state dependent instead of reloading window
- Replace placeholder texts with actual content

[Chore #158947252]

&.zoom-in {
&:hover {
animation-name: zoom-in;

Choose a reason for hiding this comment

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

Properties should be ordered animation-duration, animation-iteration-count, animation-name, animation-timing-function

.animated {
&.bounce {
&:hover {
animation-name: bounce;

Choose a reason for hiding this comment

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

Properties should be ordered animation-duration, animation-name

transform: scale(1);
}
}
.animated {

Choose a reason for hiding this comment

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

Rule declaration should be preceded by an empty line

}
50% {
transform: scale(1.1);
}

Choose a reason for hiding this comment

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

Rule declaration should be followed by an empty line

@keyframes zoom-in {
0% {
transform: scale(1);
}

Choose a reason for hiding this comment

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

Rule declaration should be followed by an empty line

.row {
flex-direction: column;
align-items: center;
}

Choose a reason for hiding this comment

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

Rule declaration should be followed by an empty line

}
// Overriding bootstrap
.row {
flex-direction: column;

Choose a reason for hiding this comment

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

Properties should be ordered align-items, flex-direction

padding: 30px 20px;
display: flex;
flex-direction: column;
/* justify-content: center; */
align-items: center;
font-size: 18px;
background-color: #242835;
color: white;

Choose a reason for hiding this comment

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

Color white should be written in hexadecimal form as #ffffff
Color literals like white should only be used in variable declarations; they should be referred to via variable everywhere else.

padding: 30px 20px;
display: flex;
flex-direction: column;
/* justify-content: center; */
align-items: center;
font-size: 18px;
background-color: #242835;

Choose a reason for hiding this comment

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

Color literals like #242835 should only be used in variable declarations; they should be referred to via variable everywhere else.

@@ -1149,12 +1151,16 @@ body {

// ABOUT US
.io-about {
height: 500px;
height: auto;

Choose a reason for hiding this comment

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

Properties should be ordered align-items, background-color, color, display, flex-direction, font-size, height, min-height, padding

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (dev@3a61c4e). Click here to learn what that means.
The diff coverage is 40%.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev      #41   +/-   ##
======================================
  Coverage       ?   84.26%           
======================================
  Files          ?       49           
  Lines          ?      623           
  Branches       ?       91           
======================================
  Hits           ?      525           
  Misses         ?       94           
  Partials       ?        4
Impacted Files Coverage Δ
client/src/components/DiscoverCenterCardRow.js 100% <ø> (ø)
client/src/components/DiscoverNavbarLoggedIn.js 100% <ø> (ø)
client/src/components/ProfileNavbar.js 50% <ø> (ø)
client/src/components/HomeShowcaseSection.js 100% <ø> (ø)
client/src/components/HomeAboutSection.js 100% <ø> (ø)
client/src/components/HomeNavbarLoggedIn.js 100% <ø> (ø)
client/src/components/DiscoverCenterCard.js 100% <ø> (ø)
client/src/containers/Discover.js 80% <ø> (ø)
client/src/components/DiscoverBody.js 100% <ø> (ø)
client/src/components/HomeNavbar.js 100% <ø> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a61c4e...d95bbdc. Read the comment docs.

@appcypher appcypher merged commit 9968259 into dev Jul 12, 2018
@appcypher appcypher changed the title #158947252 Implement feedback from Checkpoint Defence #158947252 Implement feedback Jan 30, 2019
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.

None yet

3 participants