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

Change formatting to local em-sizes #777

Merged
merged 1 commit into from Aug 26, 2018
Merged

Change formatting to local em-sizes #777

merged 1 commit into from Aug 26, 2018

Conversation

signalwerk
Copy link
Contributor

Problem: I think the component-styling looks great. But if I need to place it smaller or bigger in my design I have to change all the default stylings.

Suggested solution: if the styling is on the root-level set by rem (line: 5) and in the component by em I just need to change .DayPicker { font-size: 2rem; } and the whole calendar will scale in proportion to it.

Compatibility: This change should not break any existing styling. People who did manual styling will not be affected, since they reset the default styling. And people who made no changes will not notice, since it's visually equal to the existing codebase.

Sidenote: on Line 88 there was already a em format introduced.

Problem: I think the component-styling looks great. But if I need to place it smaller or bigger in my design I have to change all the default stylings.

Suggested solution: if the styling is on the root-level set by `rem` (line: 5) and in the component by `em` I just need to change `.DayPicker {  font-size: 2rem; }` and the whole calendar will scale in proportion to it.

Compatibility: This change should not break any existing styling. People who did manual styling will not be affected, since they reset the default styling. And people who made no changes will not notice, since it's visually equal to the existing codebase.

Sidenote: on [Line 88](https://github.com/gpbl/react-day-picker/blob/master/src/style.css#L88) there was already a `em` format introduced.
@codecov
Copy link

codecov bot commented Aug 26, 2018

Codecov Report

Merging #777 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #777   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files          15       15           
  Lines         646      646           
  Branches      142      142           
=======================================
  Hits          645      645           
  Misses          1        1

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 d616361...42ebe72. Read the comment docs.

@gpbl
Copy link
Owner

gpbl commented Aug 26, 2018

Good idea, thanks! I think we should switch completely to em (so, even the root element) in the next major release – what do you think?

@gpbl gpbl added the v:patch label Aug 26, 2018
@gpbl gpbl merged commit d91416b into gpbl:master Aug 26, 2018
@signalwerk
Copy link
Contributor Author

@gpbl that sounds good to me. I just thought I will not rock the boat too heavy 🤓 and keep it compatible with the existing codebase. but I guess it's also great to change the root to em. Feels actually better with em. Then it can be wrapped in an element with defined font-size and no need to overwrite the .DayPicker. Sounds good to me!

@signalwerk signalwerk deleted the rem-em-sizing branch August 26, 2018 11:18
@gpbl gpbl mentioned this pull request Aug 26, 2018
9 tasks
@gpbl gpbl added this to the v7.2.0 milestone Aug 27, 2018
kimamula pushed a commit to kimamula/react-day-picker that referenced this pull request Aug 17, 2022
Change formatting to local em-sizes
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