Skip to content

add task solution#1908

Open
spojrzenie wants to merge 2 commits intomate-academy:masterfrom
spojrzenie:develop
Open

add task solution#1908
spojrzenie wants to merge 2 commits intomate-academy:masterfrom
spojrzenie:develop

Conversation

@spojrzenie
Copy link

No description provided.

Copy link

@darokrk darokrk left a comment

Choose a reason for hiding this comment

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

Small fix is required for approval :)

display: grid;
justify-content: center;
gap: 48px;

Copy link

Choose a reason for hiding this comment

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

"There should be one column below 488px" think you missed this requirement :)

just add here grid-template-columns: repeat(1, 200px);

Copy link
Author

Choose a reason for hiding this comment

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

just thought this limit was not necessary as there was 1 column below 488px without this limit :)

@spojrzenie spojrzenie requested a review from darokrk June 2, 2023 19:03
Copy link

@darokrk darokrk left a comment

Choose a reason for hiding this comment

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

Nice 🥇 Give approval, but look at my last mention, test it by yourself and try to implement it :) it will improve code even more


max-width: 944px;
margin: 0 auto;
grid-template-columns: repeat(1, 200px);
Copy link

Choose a reason for hiding this comment

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

You can even try to use here 'grid-template-columns' to repeat(auto-fit, minmax(200px, 1fr)); to make the layout responsive and just leave only this breakpoint to make sure then you get max 4 columns.

@media all and (min-width: 1024px) { grid-template-columns: repeat(4, 200px); }

Then you can remove the rest breakpoints for the smaller screens.

min-width: 488px and min-width: 768px

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.

2 participants