Skip to content

Ui harmonize#41

Open
kihuha wants to merge 3 commits intodevelopfrom
ui-harmonize
Open

Ui harmonize#41
kihuha wants to merge 3 commits intodevelopfrom
ui-harmonize

Conversation

@kihuha
Copy link
Contributor

@kihuha kihuha commented May 28, 2019

What does this PR do?

Improves the UI of the entire project

Description of the task to be completed

  • Redefine cards and shadows
  • Implement masonry layout
  • Update the create article form and page

How should this be manually tested

  • Clone the repo and navigate to the root directory using a terminal
  • Install all the dependencies by typing npm install
  • Start the server by typing npm start
  • Navigate to the main page using a browser at http://localhost:3000/
  • Click on the search icon on the far right of the navigation bar to open the modal
  • Enter a search query and specify a filter then press enter to load the results (if they exist)

@kihuha kihuha temporarily deployed to ah-the-answer-frontend-s-pr-41 May 28, 2019 20:15 Inactive
}

.article-like-icons {
transition: all .2s ease;

Choose a reason for hiding this comment

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

.2 should be written with a leading zero as 0.2

}
}

.article-like-icons {

Choose a reason for hiding this comment

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

Merge rule .article-like-icons with rule on line 99

font-size: 2rem;

.featured-card {
transition: .2s ease;

Choose a reason for hiding this comment

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

Properties should be ordered backface-visibility, transition
.2 should be written with a leading zero as 0.2

margin-top: 1rem;
}

.ReactTags__remove {

Choose a reason for hiding this comment

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

Selector ReactTags__remove should be written in lowercase with hyphens

padding: 0.6rem 1rem;
outline: none;
border: none;
border: 1px solid #ced4da;

Choose a reason for hiding this comment

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

Color literals like #ced4da should only be used in variable declarations; they should be referred to via variable everywhere else.
Property border already defined on line 46

width: 30em;
padding: 0.6rem 1rem;
outline: none;
border: none;

Choose a reason for hiding this comment

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

border: 0 is preferred over border: none

margin: 0.1em;
border-radius: 30px;
padding: 0.5rem;
margin-right:0.2rem;

Choose a reason for hiding this comment

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

Colon after property should be followed by one space


.card-img-left {
width: 40%;
width: 100%;

Choose a reason for hiding this comment

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

Properties should be ordered object-fit, width

}

.navbar {
border-bottom: solid 0.5px rgba(0,0,0,0.1);

Choose a reason for hiding this comment

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

Color literals like rgba(0, 0, 0, 0.1) should only be used in variable declarations; they should be referred to via variable everywhere else.
Commas in function arguments should be followed by one space

<div className="article-list section" data-test="articleList">
{ articles && articles.slice(5).map((article) => {
<div className="article-list section card-columns" data-test="articleList">
{ articles && articles.slice(0,10).map((article) => {

Choose a reason for hiding this comment

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

A space is required after ',' comma-spacing

Copy link

@kirega kirega left a comment

Choose a reason for hiding this comment

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

The new home user interface it looking good, just as we had discussed.

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.

4 participants