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

fix: Removed unnecessary code in Grid Component #633

Merged
merged 1 commit into from
Jan 11, 2022

Conversation

RodrigoTomeES
Copy link
Contributor

Because the grid styles use Mobile First, the "@media only screen and (max-width: $ {theme.breakpoints.xs.max})" breakpoint is unnecessary. This generates the same styles and is applied in the same sizes as the ".xs" class.

Change information

I have removed the media query "@media only screen and (max-width: $ {theme.breakpoints.xs.max})". I have tested the component with and without this "media query" and I get the same results.

imagen

@RodrigoTomeES RodrigoTomeES changed the title Removed unnecessary code fix: Removed unnecessary code Sep 24, 2021
@RodrigoTomeES RodrigoTomeES changed the title fix: Removed unnecessary code fix: Removed unnecessary code in Grid Component Sep 24, 2021
@unix
Copy link
Member

unix commented Sep 27, 2021

It is indeed feasible that. After you update the commit message and snapshot, this PR will be merged.

@RodrigoTomeES
Copy link
Contributor Author

It is indeed feasible that. After you update the commit message and snapshot, this PR will be merged.

Hi, so, Do I need to change the commit message with "fix:" + my message? What is the problem of the snapshot? This changes will do by the members or by me?

@RodrigoTomeES
Copy link
Contributor Author

RodrigoTomeES commented Oct 11, 2021

It is indeed feasible that. After you update the commit message and snapshot, this PR will be merged.

@unix I just updated the git message

@ofekashery ofekashery requested a review from unix October 11, 2021 13:13
@unix unix self-assigned this Jan 11, 2022
@unix unix added the type: bug Something isn't working label Jan 11, 2022
Because the grid styles use Mobile First, the "@media only screen and (max-width: $ {theme.breakpoints.xs.max})" breakpoint is unnecessary. This generates the same styles and is applied in the same sizes as the ".xs" class.

test: update snapshots
@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #633 (dbe6f11) into master (08b966b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #633   +/-   ##
=======================================
  Coverage   99.82%   99.82%           
=======================================
  Files         194      194           
  Lines        2930     2930           
  Branches      663      663           
=======================================
  Hits         2925     2925           
  Misses          5        5           
Impacted Files Coverage Δ
components/grid/basic-item.tsx 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08b966b...dbe6f11. Read the comment docs.

@unix unix merged commit b8fd5b8 into geist-org:master Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants