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

feat(layer/zodio): adapt size #1426

Merged
merged 2 commits into from May 4, 2023
Merged

Conversation

mohamedMok
Copy link
Contributor

I have read the contributing guidelines

  • Yes
  • No

Does this PR introduce a breaking change?

  • Yes
  • No

Describe the changes

GitHub issue number or Jira issue URL: N/A

Other information

@mohamedMok mohamedMok requested a review from tiloyi May 3, 2023 10:08
@ghost ghost temporarily deployed to staging May 3, 2023 10:17 Destroyed
@ghost ghost temporarily deployed to staging May 3, 2023 10:18 Destroyed
@@ -39,28 +39,28 @@
width: 100%;

@include set-from-screen("m") {
max-width: 25rem; // 400px
max-width: $layer-max-width-m; // 400px
Copy link
Contributor

Choose a reason for hiding this comment

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

From my point of view, we don't have to put in variables, as the values are only used once, and the size does not change depending on the preset.

So we could also have done :
max-width: 25rem; // 400px => max-width: px-to-rem(400);

But the evolution made works, so that's the most important thing :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback I will make the changes before merging :)

@ghost ghost temporarily deployed to staging May 4, 2023 08:57 Destroyed
@ghost ghost temporarily deployed to staging May 4, 2023 08:59 Destroyed
@mohamedMok mohamedMok merged commit 5a26bef into feat/zodio-preset May 4, 2023
2 checks passed
@mohamedMok mohamedMok deleted the feat-layer-size branch May 4, 2023 13:38
tiloyi pushed a commit that referenced this pull request Jun 2, 2023
tiloyi pushed a commit that referenced this pull request Jun 21, 2023
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

2 participants