Feature: Category description banner. #54

sr-meh · 2018-02-27T00:00:37Z

Solves: #20

20 Category description.

Changes:

[#20] Category description banner

Adds a new banner in the category of products section.

- Adding footer to Application in a separate partial.

magmahound · 2018-02-27T00:00:52Z

theme6/source/stylesheets/site.css.scss

+          font-size: 14px;
+        }
+        .boxsocialmedia {
+          width: 95%;


Properties should be ordered align-content, display, justify-content, width

magmahound · 2018-02-27T00:00:53Z

theme6/source/stylesheets/site.css.scss

+    }
+
+    footer .information ul li a{
+      height: 22px;


Properties should be ordered align-items, color, font-family, font-size, height, line-height, margin-right, text-decoration, width

magmahound · 2018-02-27T17:00:27Z

theme6/source/stylesheets/site.css.scss

+    width: 305px;
+  }
+
+  footer .author p {


Selector should have depth of applicability no greater than 2, but was 3

magmahound · 2018-02-27T17:00:28Z

theme6/source/stylesheets/site.css.scss

+}
+
+footer .information ul li a {
+  height: 22px;


Properties should be ordered align-items, color, font-family, font-size, height, line-height, margin-right, text-decoration, width

team next time order properties - the reason for order properties alphabetically is because the searching time it's reduced, this helps you to do changes quickly in styles

magmahound · 2018-02-27T17:00:28Z

theme6/source/stylesheets/site.css.scss

+
+footer .newsletter input {
+  height: 35px;
+  width: 300px;


Properties should be ordered height, justify-content, width

magmahound · 2018-02-27T17:00:30Z

theme6/source/stylesheets/site.css.scss

+}
+
+footer .contenedornews {
+  width: 33%;


Properties should be ordered display, width

magmahound · 2018-02-27T17:00:31Z

theme6/source/stylesheets/site.css.scss

+}
+
+.sm {
+  height: 35px;


Properties should be ordered border, border-radius, height, width

magmahound · 2018-02-27T17:00:32Z

theme6/source/stylesheets/site.css.scss

+  text-align: center;
+}
+
+footer .author p {


Merge rule footer .author p with rule on line 2
Selector should have depth of applicability no greater than 2, but was 3

magmahound · 2018-02-27T17:00:34Z

theme6/source/stylesheets/site.css.scss

-.middleman-logo {
-  margin-bottom: 1rem;
-  width: 10rem;
+footer .author {


Merge rule footer .author with rule on line 2

magmahound · 2018-02-27T17:00:35Z

theme6/source/stylesheets/site.css.scss

+  display: inline-block;
+}
+
+footer .information ul {


Merge rule footer .information ul with rule on line 2
Selector should have depth of applicability no greater than 2, but was 3

magmahound · 2018-02-27T17:00:35Z

theme6/source/stylesheets/site.css.scss

+  margin-right: 50;
+}
+
+footer .information ul li {


Merge rule footer .information ul li with rule on line 2
Selector should have depth of applicability no greater than 2, but was 4

magmahound · 2018-02-27T17:00:36Z

theme6/source/stylesheets/site.css.scss

+  justify-content: center;
+}
+
+footer .information ul li a {


Merge rule footer .information ul li a with rule on line 2
Selector should have depth of applicability no greater than 2, but was 5

Ilyeo

good job team, but the next time avoid to use ul li a tags directly to give styles is much better to use a class

8geonirt · 2018-02-27T22:53:13Z

theme6/source/partials/_footer.erb

@@ -0,0 +1,40 @@
+<div class="contenedor">
+      <footer>
+          <div class="contenedorsocial">


Let's make class names more readable by using - to separate words; contenedorsocial => contenedor-social

8geonirt · 2018-02-27T22:58:02Z

theme6/source/partials/_header.erb

+ 			<div id="mySidenav" class="sidenav">
+ 				<div class="sidenav_logo">
+ 					<img class="sidenavhorse" src="../images/logo2.png">
+ 					<a href="javascript:void(0)" class="closebtn" onclick="closeNav()"><p>Close <img class="arrow_back" src="../images/Path 322 Copy [email protected]"></p></a>


Let's make this as a button, add a class or an ID on it and use that using JS to call the function closeNav();

Anchors should be used only to jump between sections/pages

8geonirt

There are some suggestions that you guys need to consider, most of them are related to sort alphabetically the properties, and the way you name the CSS classes. Besides of that you're having a good improvement...
If you have any doubts don't hesitate asking me.

8geonirt · 2018-02-27T22:58:25Z

theme6/source/partials/_header.erb

+ 			<div id="mySidenav" class="sidenav">
+ 				<div class="sidenav_logo">
+ 					<img class="sidenavhorse" src="../images/logo2.png">
+ 					<a href="javascript:void(0)" class="closebtn" onclick="closeNav()"><p>Close <img class="arrow_back" src="../images/Path 322 Copy [email protected]"></p></a>


Anchors should be used only to jump between sections/pages

8geonirt · 2018-02-27T23:01:25Z

theme6/source/stylesheets/category-description-banner.css

+  .banner_handcrafted .banner_content {
+    background: linear-gradient(90deg,
+			rgb(200, 190, 183),
+			rgb(200, 190, 183, 1) 40%,


rgb needs only three parameters while rgba needs four parameters

8geonirt · 2018-02-27T23:02:14Z

theme6/source/stylesheets/main.css

+
+  .mid-banner-saylor .mid-banner-content {
+    background: linear-gradient(90deg,
+      rgba(239, 239, 239),


Check number of parameters, rgba = 4, rgb = 3

8geonirt · 2018-02-27T23:03:27Z

theme6/source/stylesheets/site.css.scss

-    "Franklin Gothic Medium", "Century Gothic", "Liberation Sans", sans-serif;
-  padding: 18vh 1rem;
+/*==========Stylesheet DESKTOP========*/
+footer {


I'd extremely suggest to add a class to the footer of the whole page because there may be multiple footer tags between sections and not only one for the document, if there's a section with a footer it is going to have the same styles as the footer of the document

magmahound · 2018-02-28T18:24:00Z

theme6/source/javascripts/site.js

+    document.getElementById("mySidenav").style.width = "90%";
+}
+
+function closeNav() {


'closeNav' is defined but never used.

magmahound · 2018-02-28T18:24:00Z

theme6/source/javascripts/site.js

@@ -1 +1,9 @@
 // This is where it all goes :)
+
+function openNav() {


'openNav' is defined but never used.

Ilyeo

On team their should decided that naming use to css rules. With dash style-class or with low dash style_class, beside my comment LGTM

8geonirt · 2018-03-02T16:19:12Z

8geonirt

🤓👍

Ilyeo

LGTM

jaimelr and others added 26 commits February 21, 2018 13:48

Add section structure and contents

ba866f3

Add background image for banner 2

c13496b

Finish the structure of the two top banners

4060cb4

Footer Implementation

3d2414b

- Adding footer to Application in a separate partial.

Add font families link tag

fea8e63

Delete middleman default css stylesheet

9d7ae9b

Add styles to top two banners from homepage

052f8ec

Minor design improvement

4a4acfd

Add structure to two new banners

5a6d772

Add styles to new banners

0c3c754

Adding footer DESKTOP and MOBILE

96c146b

Fix typo in mid banner class name

755cb7f

Add support for tablet

096f2d5

Add desktop view

c143a59

Add footer

7f42fc9

Add footer stylesheet link

8e0d7df

Stylesheet footer

e071130

Add correct footer stylesheet

591585a

Remove fixed position in footer

a127bb6

Move content from index file to partial

c8a1cf4

[] Implement Header and Main Menu

cfe14d2

Merge branch 'featuredkits' into featured_head

978504a

Fixes_Magmahound

2023069

Fixes_Magmahound

fe607a1

Banner category description

a87d985

Add font family on the banner

3491f74

sr-meh added the Ready to Review label Feb 27, 2018

sr-meh self-assigned this Feb 27, 2018

sr-meh requested review from 8geonirt and Ilyeo February 27, 2018 00:00

magmahound reviewed Feb 27, 2018

View reviewed changes

jaimelr removed the request for review from a team February 27, 2018 01:53

jaimelr changed the base branch from master to MH-Team2 February 27, 2018 01:53

Fixes Magmahound

866ba49

magmahound reviewed Feb 27, 2018

View reviewed changes

theme6/source/stylesheets/site.css.scss Outdated

}

footer .contenedornews {

width: 33%;

Copy link

magmahound Feb 27, 2018

Choose a reason for hiding this comment

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

Properties should be ordered display, width

magmahound reviewed Feb 27, 2018

View reviewed changes

theme6/source/stylesheets/site.css.scss Outdated

}

.sm {

height: 35px;

Copy link

magmahound Feb 27, 2018

Choose a reason for hiding this comment

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

Properties should be ordered border, border-radius, height, width

magmahound reviewed Feb 27, 2018

View reviewed changes

Ilyeo reviewed Feb 27, 2018

View reviewed changes

8geonirt reviewed Feb 27, 2018

View reviewed changes

8geonirt suggested changes Feb 27, 2018

View reviewed changes

Fixes style rgba

28b7142

magmahound reviewed Feb 28, 2018

View reviewed changes

sr-meh added 2 commits February 28, 2018 12:25

Fixes style rgba

cff06ae

deletes style duplicate

2c82364

Ilyeo requested changes Mar 1, 2018

View reviewed changes

8geonirt approved these changes Mar 2, 2018

View reviewed changes

Ilyeo approved these changes Mar 2, 2018

View reviewed changes

sr-meh closed this Apr 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Category description banner. #54

Feature: Category description banner. #54

sr-meh commented Feb 27, 2018 •

edited by jaimelr

Loading

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

Ilyeo Feb 27, 2018 •

edited

Loading

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

magmahound Feb 27, 2018

Ilyeo left a comment

8geonirt Feb 27, 2018

8geonirt Feb 27, 2018

8geonirt Feb 27, 2018

8geonirt left a comment

8geonirt Feb 27, 2018

8geonirt Feb 27, 2018

8geonirt Feb 27, 2018

8geonirt Feb 27, 2018

magmahound Feb 28, 2018

magmahound Feb 28, 2018

Ilyeo left a comment

8geonirt commented Mar 2, 2018

8geonirt left a comment

Ilyeo left a comment

		@@ -1 +1,9 @@
		// This is where it all goes :)

		function openNav() {

Feature: Category description banner. #54

Feature: Category description banner. #54

Conversation

sr-meh commented Feb 27, 2018 • edited by jaimelr Loading

Solves: #20

20 Category description.

Changes:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ilyeo Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ilyeo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

8geonirt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ilyeo left a comment

Choose a reason for hiding this comment

8geonirt commented Mar 2, 2018

8geonirt left a comment

Choose a reason for hiding this comment

Ilyeo left a comment

Choose a reason for hiding this comment

sr-meh commented Feb 27, 2018 •

edited by jaimelr

Loading

Ilyeo Feb 27, 2018 •

edited

Loading