feat: catalog landing page with HTML, SCSS and BEM methodology by MikeBoltUX · Pull Request #6284 · mate-academy/layout_catalog

MikeBoltUX · 2026-06-26T00:34:41Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still explicit checklist violations that count as critical blockers for this task. The main issue is [CHECKLIST ITEM #2 – repeated sizes and special colors are put to variables]: you repeatedly hardcode 200px for grid columns in _catalog.scss, _header.scss, _price.scss, _reviews.scss, and index.scss even though $card-width: 200px; already exists, and you use #fff (and rgba(0, 0, 0, 0.1) in _catalog.scss, #34568b in _card.scss) directly instead of centralizing them into SCSS variables like $color-white and a named hover color. You also hardcode 300ms in some hover transitions (e.g., in _button.scss) instead of consistently reusing $transition-time, which is required for “all hover changes are smooth” to stay consistent across components. In addition, there are leftover tooling markers like /**24**/, /**15**/, and /**8**/ at the ends of several .scss files (_price.scss, _reviews.scss, _stars.scss, index.scss) that are not valid SCSS comments and violate the code style rules [CHECKLIST ITEM #6].

To fix this, please: 1) replace all repeated 200px literals in grid templates and media queries with $card-width, 2) introduce a $color-white: #fff; (and other needed color tokens) in your variables and use those instead of raw hex/rgba literals wherever they recur, 3) reuse $transition-time instead of hardcoding 300ms in hover transitions, and 4) remove all stray /**…**/ marker comments from your SCSS files. The rest of your implementation (semantic HTML, BEM structure, grid layout, hover behaviors, and required data-qa attributes) looks solid and close to the spec. Once these variable and code-style issues are addressed, your solution should fully meet the task requirements. Keep going—your structure and logic are on the right track, this is mostly about polishing consistency and maintainability.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

@@ -22,6 +22,399 @@
  </head>

  <body>


Same as earlier: this #fff literal in the hover state should be replaced with a color variable to comply with checklist item #2.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+  border: 1px solid transparent;
+  border-radius: 5px;      // El valor exacto de tu captura
+  cursor: pointer;
+  transition: background-color 300ms ease, color 300ms ease, border-color 300ms ease;


This hover implementation correctly changes the button background and text color, but per the requirement to have all hover changes smooth over 300ms (Checklist: All changes on :hover are smooth), consider reusing the $transition-time variable instead of hardcoding 300ms in multiple places, so timing stays consistent across components.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+  box-sizing: border-box;
+  background-color: #fff;
+  overflow: hidden;
+


This violates checklist item "Repeated sizes and special colors are put to variables": you use #fff directly here and in other styles instead of referencing a shared SCSS variable (for example a $color-white), so repeated special colors are not centralized.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+
+      /* Al hacer hover en la card, cambiamos el color del título interno */
+      .card__title {
+        color: #34568b;


Checklist item "Repeated sizes and special colors are put to variables" is partially violated: the hardcoded #34568b should be moved into a named SCSS variable (e.g. $color-card-title-hover) so that special colors are not duplicated as literals.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+    display: grid;
+
+    /* Por defecto (pantallas pequeñas): 1 columna del ancho de la card (200px) */
+    grid-template-columns: 200px;


You’re hardcoding 200px here even though you already have $card-width defined. This violates checklist item "Repeated sizes and special colors are put to variables"; reuse $card-width instead of duplicating the value.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+
+    /* 2 columnas a partir de 488px */
+    @media (min-width: 488px) {
+      grid-template-columns: repeat(2, 200px);


Same issue as above: repeat(2, 200px) duplicates the card width value. To satisfy checklist item about putting repeated sizes into variables, use the $card-width variable here.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+
+    /* 3 columnas a partir de 768px */
+    @media (min-width: 768px) {
+      grid-template-columns: repeat(3, 200px);


And here again, repeat(3, 200px) repeats the literal card width instead of using $card-width, which goes against the checklist rule for centralizing repeated sizes.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+
+    /* 4 columnas a partir de 1024px */
+    @media (min-width: 1024px) {
+      grid-template-columns: repeat(4, 200px);


This is another repetition of 200px for card width. Consider repeat(4, $card-width) to align with the "Repeated sizes and special colors are put to variables" requirement.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

@@ -0,0 +1,31 @@
+.catalog {
+    /* Paddings fijos requeridos: 50px vertical, 40px horizontal */
+    padding: 50px 40px;


The hardcoded #fff header background is a design token; to comply with the checklist requirement about variables, consider moving this into a SCSS variable (e.g. $color-background or $color-white) instead of using the literal.

mateacademy-ai-mentor · 2026-06-26T00:36:34Z

+.catalog {
+    /* Paddings fijos requeridos: 50px vertical, 40px horizontal */
+    padding: 50px 40px;
+


rgba(0, 0, 0, 0.1) is another special color value that would be better as a variable (e.g. $color-border-header) to satisfy the checklist about repeated/special colors being stored in variables, especially if you reuse this elsewhere.

mateacademy-ai-mentor · 2026-06-26T00:36:37Z

+
+    /* 3 columnas a partir de 768px */
+    @media (min-width: 768px) {
+      grid-template-columns: repeat(3, 200px);


This trailing /**24**/ marker is not valid SCSS and appears to be a leftover artifact. It violates the general code-style cleanliness requirement; remove it to keep the stylesheet valid.

mateacademy-ai-mentor · 2026-06-26T00:36:37Z

+    gap: 46px 48px;
+
+    /* Centramos el grid en la pantalla para que se vea bien */
+    justify-content: center;


Same as in _price.scss: this /**15**/ looks like a leftover marker rather than a real comment. Remove it to comply with the "Code follows all the Code Style Rules" checklist item and avoid confusing, non-SCSS syntax.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

+    &__container {
+      max-width: 1200px;
+      margin: 0 auto;
+      padding: 0 16px;


Here and in all following grid-template-columns you hardcode 200px even though $card-width is defined; this violates checklist item #2: "Repeated sizes and special colors are put to variables". Use $card-width instead of the literal 200px.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

+  .nav {
+    text-transform: uppercase;
+
+    &__list {


Same checklist item #2 applies here: replace repeat(2, 200px) with repeat(2, $card-width) to avoid repeating the literal width.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

+      align-items: center;
+      list-style: none;
+      margin: 0;
+      padding: 0;


For consistency with variables (checklist #2), this should use $card-width rather than repeating 200px again.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

+
+    &__item {
+      margin-left: 24px;
+


Likewise, change repeat(4, 200px) to repeat(4, $card-width) to keep the card width centralized in a variable.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

@@ -0,0 +1,62 @@
+.header {
+    width: 100%;
+    background-color: #fff;


#fff is a special color used in multiple components (header, buttons, etc.); per checklist item #2, move this into a shared SCSS variable (e.g. $color-white) instead of hardcoding it.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

+      align-items: center;
+      list-style: none;
+      margin: 0;
+      padding: 0;


This trailing /**24**/ looks like a leftover marker, not a meaningful comment; cleaning it up will help align with the code-style guideline checklist item: "Code follows all the Code Style Rules".

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

+      align-items: center;
+    }
+  }
+


Same as in _price.scss, this stray /**15**/ marker at the end of the file isn’t a normal SCSS comment and should be removed to keep the stylesheet clean and compliant with the style rules.

mateacademy-ai-mentor · 2026-06-26T00:36:38Z

@@ -0,0 +1,23 @@
+@import "./variables";
+
+.price {


Since the checklist requires that “Repeated sizes and special colors are put to variables”, consider replacing the literal #fff with a named SCSS variable (e.g. $color-white) so it’s centralized and reused consistently.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

+  justify-content: space-between;
+
+  &__label { // O &__label
+    font-family: $font-main;


You use the fixed card width 200px multiple times across the layout; to fully follow the checklist about repeated sizes, it would be better to replace this literal with the existing $card-width variable (or create one if needed) rather than repeating the raw value.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

+  &__value { // O &__value
+    font-family: $font-main;
+    font-size: 18px;
+    font-weight: 700;


For the same reason as above (checklist: repeated sizes to variables), these repeated 200px values in the media queries should use the shared width variable instead of the literal.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

+    line-height: 1.5;
+    color: $color-text-secondary; // Ese #707070 que suele ser el gris de la guía
+  }
+


Same as in _price.scss, this /**15**/ marker doesn’t belong in the SCSS source and may confuse future readers; consider removing it to satisfy the general code style guideline.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

@@ -0,0 +1,14 @@
+@import "./variables";
+
+.reviews {


This hardcoded #fff is a “special color” that appears in multiple places; it violates checklist item #2: "Repeated sizes and special colors are put to variables". Consider defining a $color-white (or similar) variable and using that instead of the literal.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

+  justify-content: space-between;          // Aquí va el gap exacto de Figma
+
+  &__count {
+    font-size: 10px;


Here you hardcode 200px while you already have $card-width defined in your variables. This violates checklist item #2: "Repeated sizes and special colors are put to variables"; use $card-width instead of duplicating the numeric value.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

+  gap: 4px;
+
+  &__item {
+    display: inline-block;


This tooling marker /**8**/ at the end of the variables file isn’t valid SCSS and should be removed to comply with the checklist item “Code follows all the Code Style Rules”.

mateacademy-ai-mentor · 2026-06-26T00:36:41Z

+@import 'button';
+@import 'card';
+@import 'price';
+@import 'reviews';


Since $card-width: 200px; is already defined, this hardcoded 200px should be replaced with $card-width to satisfy checklist item: "Repeated sizes and special colors are put to variables".

mateacademy-ai-mentor · 2026-06-26T00:36:42Z

-}
+@import 'variables';
+@import 'base';
+@import 'header';


This literal #fff is a special color used in multiple components (e.g., header background, button hover). To comply with the checklist item about variables, consider defining a $color-white (or similar) and using that variable here.

mateacademy-ai-mentor · 2026-06-26T00:36:42Z

+/* Componentes de la Card */
+@import 'button';
+@import 'card';
+@import 'price';


This trailing /**8**/ marker is not part of valid SCSS and looks like a leftover from tooling. It should be removed to keep the variables file clean and aligned with the code style rules.

feat: catalog landing page with HTML, SCSS and BEM methodology

37ebfda

mateacademy-ai-mentor suggested changes Jun 26, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Jun 26, 2026

View reviewed changes

Uh oh!

Conversation

MikeBoltUX commented Jun 26, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants